[Nouveau] [PATCH 1/5] drm: add optional per device rwsem for all ioctls

[Marcin Slusarz]

On Mon, Apr 23, 2012 at 09:51:48AM +0200, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 12:18:28AM +0200, Marcin Slusarz wrote:
> > Nouveau, in normal circumstances, does not need device lock for every ioctl,
> > but incoming "gpu reset" code needs exclusive access to the device.
> > This commit adds drm_driver flag which turns on read lock ioctl encapsulation.
> > 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
> 
> Why can't we just move this down to nouveau driver code?

I can't wrap drm core ioctls this way and adding locks at driver entry points
would be at best unmaintainable, at worst - impossible (some of them are
probably called in atomic context).

> I really hate adding random stuff to drm core that other drivers can't opt
> out of - all the dri1 nightmares are made of these essentially.

It's enabled only when driver requested it. I'm not sure what dri1 nightmares
you are talking about - I wasn't involved in dri1. Can you elaborate?

> And radeon
> and i915 seem to be able to reset without adding anything to drm core.

Both radeon and i915 take per-device locks (radeon_device->cs_mutex for radeon,
drm_device->struct_mutex for i915), which nouveau does not need. They might be
bottlenecks some day.

> So why can't nouveau do the same? Also, if this is indeed necessary and we
> add this as some mandatory core infrastructure, it's not good enough: In
> i915 we go to great lengths to ensure that all processes waiting for gpu
> reset are still interruptible, in case the gpu reset dies unexpectedly
> (which happens), so this would need to be improved.

Well, I don't like non-interruptability of rwsems too, but I don't see any
other locking primitive which is both interruptible and have read-write
semantics. I was hoping someone would point me at one.

How gpu reset can "die unexpectedly"?

Marcin