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)
> {
>
>
