On 20/05/20(Wed) 12:28, Mark Kettenis wrote:
> > Date: Wed, 20 May 2020 11:42:32 +0200
> > From: Martin Pieuchot <[email protected]>
> >
> > Diff below fixes an incoherency between poll(2) and kqueue(2) when it
> > comes to non-character devices. It makes spec_kqfilter() behaves like
> > spec_poll(): returns "true" when applied to any non-character device.
> >
> > ok?
>
> That is a change in behaviour. What do other BSDs do in this case?
By reading code I figured out that:
- NetBSD only calls the kqfilter handler for character devices and
returns EOPNOTSUPP otherwise.
- FreeBSD doesn't allow opening block devices in devfs_open(), so
only character devices seem to support poll(2) and kqueue(2).
- I couldn't find a difference between block and character devices
handling in DragonFlyBSD, it seems that the underlying kqfilter
handler is always called.
> I think the new behaviour makes sense, but it could reveal some nasty
> bugs in cases where code accidantily uses kqueue(2) with a file
> descriptor for a block device.
The alternative I could think of was changing the existing poll handler
to do selfalse() instead of seltrue(). However this makes it very hard
to figure out a regression. On the other hand, assuming that a block
device is always ready to be read via kqueue(2) seems to have a low
impact.
> To achieve your goal, d_kqfilter() will become non-optional in the
> future isn't it?
It should become the replacement for the existing d_poll() functions,
that's why I'm trying to make sure their behavior are coherent.
> > Index: kern/spec_vnops.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/spec_vnops.c,v
> > retrieving revision 1.100
> > diff -u -p -r1.100 spec_vnops.c
> > --- kern/spec_vnops.c 20 Jan 2020 23:21:55 -0000 1.100
> > +++ kern/spec_vnops.c 20 May 2020 09:36:48 -0000
> > @@ -386,11 +386,9 @@ spec_poll(void *v)
> > dev_t dev;
> >
> > switch (ap->a_vp->v_type) {
> > -
> > default:
> > return (ap->a_events &
> > (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM));
> > -
> > case VCHR:
> > dev = ap->a_vp->v_rdev;
> > return (*cdevsw[major(dev)].d_poll)(dev, ap->a_events, ap->a_p);
> > @@ -400,12 +398,17 @@ int
> > spec_kqfilter(void *v)
> > {
> > struct vop_kqfilter_args *ap = v;
> > -
> > dev_t dev;
> >
> > dev = ap->a_vp->v_rdev;
> > - if (cdevsw[major(dev)].d_kqfilter)
> > - return (*cdevsw[major(dev)].d_kqfilter)(dev, ap->a_kn);
> > +
> > + switch (ap->a_vp->v_type) {
> > + default:
> > + return seltrue_kqfilter(dev, ap->a_kn);
> > + case VCHR:
> > + if (cdevsw[major(dev)].d_kqfilter)
> > + return (*cdevsw[major(dev)].d_kqfilter)(dev, ap->a_kn);
> > + }
> > return (EOPNOTSUPP);
> > }
> >
> >
> >
