* 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
