Thank you Marco :-) -- CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
On Mon, Oct 19, 2026 at 6:57 PM Marco C. <[email protected]> wrote: > > Hello Tomek, > > It is done. The references are in the commit message, PR body and in > the documentation. The link is on PR body and documentation. > > Thank you :-) > > Marco Casaroli > > On Mon, Oct 19, 2026 at 4:46 PM Tomek CEDRO <[email protected]> wrote: > > > > Thank you Greg! :-) > > > > I would love to see a reference placed somewhere in the commit > > messages / documentation as "IEEE Std 1003.1-2017 (Revision of IEEE > > Std 1003.1-2008) / The Open Group Base Specifications Issue 7, 2018 > > edition" :-) > > > > -- > > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > > > > On Mon, Oct 19, 2026 at 1:18 PM Gregory Nutt <[email protected]> wrote: > > > > > > Should be > > > https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/poll.h.html > > > > > > ________________________________ > > > From: Tomek CEDRO <[email protected]> > > > Sent: Monday, October 19, 2026 5:35 AM > > > To: [email protected] <[email protected]> > > > Subject: Re: [DISCUSS] struct pollfd with the POSIX layout (breaking > > > change) > > > > > > * Thank you @casaroli good work! :-) > > > * Could you please provide reference to the exact POSIX standard > > > version and location that defines correct structures? :-) > > > * We need some real world hardware runtime testing :-) > > > > > > -- > > > CeDeROM, SQ7MHZ, http://www.tomek.cedro.info > > > > > > On Sun, Oct 18, 2026 at 11:28 PM Marco C. <[email protected]> > > > wrote: > > > > > > > > Hello all, > > > > > > > > The draft pull request is open: > > > > > > > > https://github.com/apache/nuttx/pull/20796 > > > > > > > > It is stacked on two cleanup pull requests: > > > > > > > > https://github.com/apache/nuttx/pull/20794 nxstyle errors in 60 of > > > > the touched files (whitespace and formatting only) > > > > https://github.com/apache/nuttx/pull/20795 fusb302.c: two > > > > unbalanced parentheses (it did not compile with > > > > CONFIG_I2C_RESET), and its nxstyle errors > > > > > > > > When they merge and #20796 is rebased, it becomes easier to review. > > > > The relevant commits of #20796 are the last 3: > > > > > > > > !fs/poll: Use struct pollfd_s for the kernel side of poll. > > > > !fs/poll: Give struct pollfd the POSIX layout. > > > > Documentation: Describe struct pollfd and struct pollfd_s. > > > > > > > > One change since my first mail: the rename commit is also marked with > > > > "!" and "BREAKING CHANGE", because it changes the poll method of > > > > out-of-tree drivers. > > > > > > > > The two nuttx-apps pull requests: > > > > > > > > https://github.com/apache/nuttx-apps/pull/3878 PRIx32 formats > > > > https://github.com/apache/nuttx-apps/pull/3879 rpmsgdev test driver > > > > > > > > I also ran the test program on real hardware, two architectures, > > > > flat builds: > > > > > > > > Raspberry Pi RP2350 (Cortex-M33), pimoroni-pico-2-plus:nsh > > > > > > > > master: the layout checks FAIL. poll() on 3 entries with the > > > > standard layout returns 1 and does not write revents. The > > > > ppoll() timeout check FAILs. ostest: status 0. > > > > #20796: all checks PASS. ostest: status 0. > > > > > > > > ESP32-S3 (Xtensa LX7), esp32s3-devkit:nsh > > > > > > > > master: the layout checks FAIL, and the test with two threads that > > > > poll the same array crashes the board: > > > > > > > > xtensa_user_panic: User Exception: EXCCAUSE=001d > > > > task: pollabi > > > > PC: 42025cda VADDR: 00000000 > > > > pipecommon_poll at drivers/pipes/pipe_common.c:773 > > > > file_poll at fs/vfs/fs_poll.c:352 > > > > > > > > EXCCAUSE 0x1d is a store to address 0. In a flat build, > > > > poll() keeps its state (arg, cb, priv) in the caller's > > > > array. When two threads poll the same array, the teardown > > > > of one thread clears the state that the other thread still > > > > uses, and the pipe driver then writes through a NULL > > > > pointer. > > > > > > > > #20796: all checks PASS, no crash. poll() now works on its own > > > > kernel copy of the array in every build mode. > > > > > > > > On this ESP32-S3, ostest stops with an assertion in its libc tests, > > > > on master and with #20796 alike. That is an older failure on this > > > > board, not related to poll(). > > > > > > > > The pull request has the test tables. > > > > > > > > Regards, > > > > Marco Casaroli > > > > > > > > On Mon, Oct 19, 2026 at 1:02 AM Marco C. <[email protected]> > > > > wrote: > > > > > > > > > > Hello all, > > > > > > > > > > I want to change struct pollfd in include/sys/poll.h to the POSIX > > > > > layout. This is a breaking change, so I ask for your comments before > > > > > the pull request leaves the draft state. > > > > > > > > > > 1. The problem > > > > > > > > > > Today struct pollfd has 32-bit events and revents and three internal > > > > > fields (arg, cb and priv). It is 40 bytes on a 64-bit target and 24 > > > > > bytes on a 32-bit target. POSIX gives it three fields: int fd, short > > > > > events and short revents (8 bytes). > > > > > > > > > > - A program or a library that builds its own array with the standard > > > > > layout and calls poll() gets its neighbouring entries overwritten. > > > > > Language runtimes, for example the Rust libc crate, already use the > > > > > 8-byte layout for NuttX. > > > > > - In flat and protected builds, poll() uses the caller's array > > > > > directly. The kernel writes a semaphore address and a callback > > > > > pointer into it, and poll_notify() calls that callback, often from > > > > > an > > > > > interrupt handler. In a protected build the array is user memory, so > > > > > the kernel keeps a callback in memory that user code can change. > > > > > - Two threads cannot poll the same array at the same time (see the > > > > > REVISIT comment in poll.h). > > > > > > > > > > 2. The change > > > > > > > > > > - struct pollfd gets the POSIX layout: int fd; short events; > > > > > short revents. > > > > > - A new kernel header, include/nuttx/poll.h, holds the kernel type > > > > > struct pollfd_s (fd, 32-bit events and revents, arg, cb, priv), > > > > > pollevent_t, pollcb_t, POLLALWAYS, poll_notify() and > > > > > poll_default_cb(). sys/poll.h keeps only the POSIX part. > > > > > - poll() copies the fd and events of each entry into a struct pollfd_s > > > > > array and copies revents back. Up to 4 entries go on the stack, more > > > > > go on the fs heap. The kernel build already copied the array on > > > > > every > > > > > call. > > > > > - The poll() method of drivers and sockets, file_poll(), psock_poll() > > > > > and poll_notify() take struct pollfd_s. This is a mechanical rename > > > > > in about 150 files. > > > > > - select(), ppoll(), pselect(), epoll and the system call interface do > > > > > not change. epoll already keeps its own kernel entry for each file > > > > > descriptor. > > > > > > > > > > 3. Cost > > > > > > > > > > On qemu-armv8a the code grows by 168 bytes (flat build) and 48 bytes > > > > > (kernel build). The stack of poll() grows by 160 bytes on a 64-bit > > > > > target and by 96 bytes on a 32-bit target. > > > > > > > > > > 4. Effect on users > > > > > > > > > > - Applications: rebuild. Source code that uses only fd, events and > > > > > revents does not change. Code that prints events or revents with > > > > > PRIx32 must use %x (one nuttx-apps pull request fixes the five > > > > > programs in nuttx-apps). > > > > > - Out-of-tree drivers: rename struct pollfd to struct pollfd_s in the > > > > > poll methods, the waiter arrays and the poll callbacks. Include > > > > > <nuttx/poll.h> if the file does not include <nuttx/fs/fs.h>. The > > > > > compiler reports every place that is not renamed. > > > > > > > > > > 5. Pull requests > > > > > > > > > > - One pull request with three commits: the rename (no change in > > > > > behaviour), the layout change (marked with "!" and "BREAKING > > > > > CHANGE"), and the documentation. > > > > > - Two nuttx-apps pull requests: the PRIx32 formats, and the rpmsgdev > > > > > test driver. > > > > > > > > > > Tests: a test program checks the size of struct pollfd and polls an > > > > > array with the standard layout (pipes, ppoll(), select(), epoll, two > > > > > threads on one array). ostest passes on qemu-armv8a:knsh and > > > > > qemu-armv8a:nsh. Help with tests on real hardware is welcome. > > > > > > > > > > Please comment here before I open the draft pull request, or in > > > > > the draft pull request, that comes after the whitespace and > > > > > formatting preparation PRs. > > > > > > > > > > Regards, > > > > > Marco Casaroli
