On Mon, Apr 27, 2020 at 04:52:33PM +0200, Martin Pieuchot wrote: > Diff below extends the existing drmkqfilter() to support EVFILT_READ. > This makes drm(4)'s kqueue support in pair with poll(). > > The event list queried in the filt_drmread() should be protected by the > `event_lock' mutex. This could be done by using the `kdev' backpointer > as shown in comment. However this would require some plumbing to make > use of `minor'. The side effect of this approach would be to reduce the > diff with Linux.
You seem to be confusing kdev and dev in the drm_minor struct. at least in linux kdev is 'struct device *' and dev is 'struct drm_device *' (which has the event lock). I have a tree which uses the drm_minor struct but it is part of a much larger diff. Could you explain what prompted this diff? > > I'd be interested to hear if somebody sees a different approach. > > That said, as long as the KERNEL_LOCK() is taken in all these code paths > it should be safe to commit the filter as it is. > > Comments, Oks? > > Index: sys/conf.h > =================================================================== > RCS file: /cvs/src/sys/sys/conf.h,v > retrieving revision 1.150 > diff -u -p -r1.150 conf.h > --- sys/conf.h 21 Apr 2020 08:29:27 -0000 1.150 > +++ sys/conf.h 27 Apr 2020 14:43:48 -0000 > @@ -439,7 +439,7 @@ extern struct cdevsw cdevsw[]; > (dev_type_stop((*))) enodev, 0, selfalse, \ > (dev_type_mmap((*))) enodev } > > -/* open, close, read, ioctl, poll, mmap, nokqfilter */ > +/* open, close, read, ioctl, poll, mmap, kqfilter */ > #define cdev_drm_init(c,n) { \ > dev_init(c,n,open), dev_init(c,n,close), dev_init(c, n, read), \ > (dev_type_write((*))) enodev, dev_init(c,n,ioctl), \ > Index: dev/pci/drm/drm_drv.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v > retrieving revision 1.174 > diff -u -p -r1.174 drm_drv.c > --- dev/pci/drm/drm_drv.c 7 Apr 2020 13:27:51 -0000 1.174 > +++ dev/pci/drm/drm_drv.c 27 Apr 2020 14:43:48 -0000 > @@ -484,6 +484,30 @@ filt_drmkms(struct knote *kn, long hint) > return (kn->kn_fflags != 0); > } > > +void > +filt_drmreaddetach(struct knote *kn) > +{ > + struct drm_file *file_priv = kn->kn_hook; > + int s; > + > + s = spltty(); > + klist_remove(&file_priv->rsel.si_note, kn); > + splx(s); > +} > + > +int > +filt_drmread(struct knote *kn, long hint) > +{ > + struct drm_file *file_priv = kn->kn_hook; > + int val = 0; > + > + //mtx_enter(&file_priv->minor->kdev->event_lock); > + val = !list_empty(&file_priv->event_list); > + //mtx_leave(&file_priv->minor->kdev->event_lock); > + > + return (val); > +} > + > const struct filterops drm_filtops = { > .f_flags = FILTEROP_ISFD, > .f_attach = NULL, > @@ -491,30 +515,51 @@ const struct filterops drm_filtops = { > .f_event = filt_drmkms, > }; > > +const struct filterops drmread_filtops = { > + .f_flags = FILTEROP_ISFD, > + .f_attach = NULL, > + .f_detach = filt_drmreaddetach, > + .f_event = filt_drmread, > +}; > + > int > drmkqfilter(dev_t kdev, struct knote *kn) > { > struct drm_device *dev = NULL; > - int s; > + struct drm_file *file_priv = NULL; > + int s; > > dev = drm_get_device_from_kdev(kdev); > if (dev == NULL || dev->dev_private == NULL) > return (ENXIO); > > switch (kn->kn_filter) { > + case EVFILT_READ: > + mutex_lock(&dev->struct_mutex); > + file_priv = drm_find_file_by_minor(dev, minor(kdev)); > + mutex_unlock(&dev->struct_mutex); > + if (file_priv == NULL) > + return (ENXIO); > + > + kn->kn_fop = &drmread_filtops; > + kn->kn_hook = file_priv; > + > + s = spltty(); > + klist_insert(&file_priv->rsel.si_note, kn); > + splx(s); > + break; > case EVFILT_DEVICE: > kn->kn_fop = &drm_filtops; > + kn->kn_hook = dev; > + > + s = spltty(); > + klist_insert(&dev->note, kn); > + splx(s); > break; > default: > return (EINVAL); > } > > - kn->kn_hook = dev; > - > - s = spltty(); > - klist_insert(&dev->note, kn); > - splx(s); > - > return (0); > } > > @@ -772,7 +817,6 @@ out: > return (gotone); > } > > -/* XXX kqfilter ... */ > int > drmpoll(dev_t kdev, int events, struct proc *p) > { > >