Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] proc: dual poll_wait() calls in proc_reg_poll() break AIO poll --
commit 443c1e3f3b75bcf766632cccb5651e0034423048 Author: Christian Brauner <[email protected]> proc: own the wait queue of an entry with a ->proc_poll() This commit gives procfs entries that implement ->proc_poll() a procfs-owned wait queue in struct pde_open_node and adds proc_poll_notify() to wake pollers on that queue. proc_reg_poll() calls poll_wait() on the procfs wait queue before invoking the entry's ->proc_poll() callback with the same poll_table. > diff --git a/fs/proc/inode.c b/fs/proc/inode.c > index 59de4681d8452..ab30a58879d51 100644 > --- a/fs/proc/inode.c > +++ b/fs/proc/inode.c [ ... ] > @@ -555,26 +561,46 @@ static ssize_t proc_reg_write(struct file *file, const > char __user *buf, size_t > return rv; > } > > -static __poll_t pde_poll(struct proc_dir_entry *pde, struct file *file, > struct poll_table_struct *pts) > -{ > - const auto poll = pde->proc_ops->proc_poll; > - if (poll) > - return poll(file, pts); > - return DEFAULT_POLLMASK; > -} > - > +/* > + * The pollers of an entry sleep on a wait queue procfs owns, which is there > + * for as long as the entry's inode is, however the entry's owner goes away. > + * The owner wakes them through proc_poll_notify(), and procfs does when the > + * entry is removed: a poll of a removed entry reports EPOLLERR | EPOLLPRI, > + * like kernfs_fop_poll(). > + */ > static __poll_t proc_reg_poll(struct file *file, struct poll_table_struct > *pts) > { > struct proc_dir_entry *pde = PDE(file_inode(file)); > __poll_t rv; > > - if (!pde_pin(pde)) > + if (!pde_has_proc_poll(pde)) > return DEFAULT_POLLMASK; > - rv = pde_poll(pde, file, pts); > + poll_wait(file, &pde->open->poll, pts); > + if (!pde_pin(pde)) > + return DEFAULT_POLLMASK | EPOLLERR | EPOLLPRI; > + rv = pde->proc_ops->proc_poll(file, pts); [Severity: Medium] Does calling poll_wait() on pde->open->poll in proc_reg_poll() before passing pts to pde->proc_ops->proc_poll() break Linux AIO polling (IOCB_CMD_POLL via io_submit()) for unconverted procfs entries? While ->proc_poll() implementations still receive pts and call poll_wait() on their own wait queues, vfs_poll() -> proc_reg_poll() ends up calling poll_wait() twice on the same poll_table_struct: first on &pde->open->poll and then on the subsystem's wait queue inside ->proc_poll(). When userspace submits an IOCB_CMD_POLL request via io_submit() on a pollable procfs file (such as /proc/pressure/*, /proc/mdstat, /proc/swaps, or /proc/kmsg), aio_poll() sets apt.pt._qproc = aio_poll_queue_proc. The first poll_wait() queues the request on &pde->open->poll and sets pt->queued = true, and the second poll_wait() inside ->proc_poll() hits the single-waitqueue check in fs/aio.c:aio_poll_queue_proc(): /* multiple wait queues per file are not supported */ if (unlikely(pt->queued)) { pt->error = -EINVAL; return; } If the polled event is not immediately ready, aio_poll() sees apt.error == -EINVAL, removes the wait entry from &pde->open->poll, and fails the io_submit() request with -EINVAL. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=22
