> Date: Tue, 28 Apr 2020 11:40:17 +0200 > From: Martin Pieuchot <m...@openbsd.org> > > On 27/04/20(Mon) 19:34, Martin Pieuchot wrote: > > On 28/04/20(Tue) 01:54, Jonathan Gray wrote: > > > 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. > > > > Thanks for pointing that out! Do you see another way to comply to the > > locking then? > > If not or if we agree that this can be improved later on I'd like to > commit the fixed diff below, ok?
Please don't commit anything with C++ style comments. I'm also not convinced that this is the appropriate way to implement kqueue for drm devices. This is Linux-compat stuff and Linux doesn't have kqueue. > 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 28 Apr 2020 09:37:31 -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->dev->event_lock); > + val = !list_empty(&file_priv->event_list); > + //mtx_leave(&file_priv->minor->dev->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) > { > >