On Wed, Sep 30, 2026 at 05:50:58AM -0400, Hengyu Liang wrote: > On Fri, Sep 25, 2026 at 5:59 AM Joe Damato <[email protected]> wrote: > > > > On Thu, Sep 24, 2026 at 02:57:47PM -0400, [email protected] wrote: > > [...] > > > > default: > > > - ret = -EINVAL; > > > + ret = -ENOIOCTLCMD; > > > break; > > > } > > > > I think based on the documentation this is probably right, but I am now > > wondering why both ep_eventpoll_ioctl and ep_eventpoll_bp_ioctl need to > > exist. > > > > Maybe when I first implemented this I thought it made sense to factor > > out the busy poll ioctls into their own function, but in retrospect maybe > > it's cleaner to just collapse the ioctl function into a single one > > instead of having two layers? > > > > In other words, maybe: > > - delete ep_eventpoll_ioctl > > - add the is_file_epoll check to ep_eventpoll_bp_ioctl > > - rename ep_eventpoll_bp_ioctl to ep_eventpoll_ioctl > > - fix the test (as you did in this version of the patch) > > > > Would result in a cleaner fewer helpers / cleaner code ? > > Thanks for taking a look. Agreed, a single handler would be cleaner. > Two things I noticed while looking into it: > > 1. With CONFIG_NET_RX_BUSY_POLL=n, ep_eventpoll_bp_ioctl() is the stub > that returns -EOPNOTSUPP for every command. If it became the > .unlocked_ioctl handler as is, every ioctl on an epoll fd would fail > with EOPNOTSUPP on those kernels, which is the same problem in a > different config. So the stub would need to keep a small switch: > > static long ep_eventpoll_ioctl(struct file *file, unsigned int cmd, > unsigned long arg) > { > switch (cmd) { > case EPIOCSPARAMS: > case EPIOCGPARAMS: > return -EOPNOTSUPP; > default: > return -ENOIOCTLCMD; > } > }
Yea, I see. I read it more closely this time. I suspect when I wrote this originally, I had factored the code this way that way the stub could take care of the CONFIG_NET_RX_BUSY_POLL=n case. So, now I'm not sure it make sense to fold them into a single handler becauase we'd end up duplicating the switch logic in two places for each CONFIG_NET_RX_BUSY_POLL setting. So, in retrospect, I think probably the original patch you proposed might be the best option. Sorry if I misread it the first time.
