[Intel-gfx] [PATCH] drm/ioctl: Ditch DRM_UNLOCKED except for the legacy vblank ioctl

[Daniel Vetter]

On Wed, Jun 5, 2019 at 12:50 PM Michel Dänzer <michel at daenzer.net> wrote:
>
> On 2019-05-29 11:30 a.m., Daniel Vetter wrote:
> > This completes Emil's series of removing DRM_UNLOCKED from modern
> > drivers. It's entirely cargo-culted since we ignore it on
> > non-DRIVER_LEGACY drivers since:
> >
> > commit ea487835e8876abf7ad909636e308c801a2bcda6
> > Author: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Date:   Mon Sep 28 21:42:40 2015 +0200
> >
> >     drm: Enforce unlocked ioctl operation for kms driver ioctls
> >
> > Now justifying why we can do this for legacy drives too (and hence
> > close the source of all the bogus copypasting) is a bit more involved.
> > DRM_UNLOCKED was introduced in:
> >
> > commit ed8b67040965e4fe695db333d5914e18ea5f146f
> > Author: Arnd Bergmann <arnd at arndb.de>
> > Date:   Wed Dec 16 22:17:09 2009 +0000
> >
> >     drm: convert drm_ioctl to unlocked_ioctl
> >
> > As a immediate hack to keep i810 happy, which would have deadlocked
> > without this trickery. The old BKL is automatically dropped in
> > schedule(), and hence the i810 vs. mmap_sem deadlock didn't actually
> > cause a real deadlock. But with a mutex it would. The solution was to
> > annotate these as DRM_UNLOCKED and mark i810 unsafe on SMP machines.
> >
> > This conversion caused a regression, because unlike the BKL a mutex
> > isn't dropped over schedule (that thing again), which caused a vblank
> > wait in one thread to block the entire desktop and all its apps. Back
> > then we did vblank scheduling by blocking in the client, awesome isn't
> > it. This was fixed quickly in (ok not so quickly, took 2 years):
> >
> > commit 8f4ff2b06afcd6f151868474a432c603057eaf56
> > Author: Ilija Hadzic <ihadzic at research.bell-labs.com>
> > Date:   Mon Oct 31 17:46:18 2011 -0400
> >
> >     drm: do not sleep on vblank while holding a mutex
> >
> > All the other DRM_UNLOCKED annotations for all the core ioctls was
> > work to reach finer-grained locking for modern drivers. This took
> > years, and culminated in:
> >
> > commit fdd5b877e9ebc2029e1373b4a3cd057329a9ab7a
> > Author: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Date:   Sat Dec 10 22:52:54 2016 +0100
> >
> >     drm: Enforce BKL-less ioctls for modern drivers
> >
> > DRM_UNLOCKED was never required by any legacy drivers, except for the
> > vblank_wait IOCTL. Therefore we will not regress these old drivers by
> > going back to where we've been in 2011. For all modern drivers nothing
> > will change.
> >
> > To make this perfectly clear, also add a comment to DRM_UNLOCKED.
> >
> > Cc: Emil Velikov <emil.l.velikov at gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 139 ++++++++++++++++++------------------
> >  include/drm/drm_ioctl.h     |   3 +
> >  2 files changed, 72 insertions(+), 70 deletions(-)
>
> Did you miss drivers/gpu/drm/drm_ioc32.c ?

Hm indeed. Not a worry from a copypasta-spreading pov, since hopefully
no one copypastes ioc32 compat code. But from a consistency side
should definitely make sure ioctls work the same whether userspace is
32bit or 64bit. I'll respin, thanks for spotting this.
-Daniel

>
>
> --
> Earthling Michel Dänzer               |              https://www.amd.com
> Libre software enthusiast             |             Mesa and X developer



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch