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

[Emil Velikov]

On Wed, 5 Jun 2019 at 13:08, Daniel Vetter <daniel.vetter at ffwll.ch> 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.
>
> v2: Don't forget about drm_ioc32.c (Michel). Not a source of copypasta
> mistakes when creating driver ioctl tables, but better to be
> consistent.
>
Personally I would omit the "Not a source of copupasta..." note.

> Cc: Michel Dänzer <michel at daenzer.net>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

Double-checked that only the VBLACK ioctl retained its annotation and
all other core ioctls are DRM_UNLOCKED free.
Technically with this change UMS drivers will start using a lock on
the listed ioctls. I do not expect this to be a problem and admittedly
I did not audit existing userspace.

That said, the patch looks reasonable:
Acked-by: Emil Velikov <emil.velikov at collabora.com>

Daniel, can you apply any outstanding DRM_UNLOCKED patches from my series?

Thanks
Emil