Op 10-09-15 om 11:51 schreef Daniel Vetter:
> On Tue, Sep 08, 2015 at 03:26:47PM +0200, Thierry Reding wrote:
>> From: Thierry Reding <treding@xxxxxxxxxx>
>>
>> These functions are like drm_modeset_{,un}lock_all(), but they take the
>> lock acquisition context as a parameter rather than storing it in the
>> DRM device's drm_mode_config structure.
>>
>> Implement drm_modeset_{,un}lock_all() in terms of the new function for
>> better code reuse, and add a note to the kerneldoc that new code should
>> use the new functions.
>>
>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> For the record quick summary of what I've mentioned on irc:
> - lock_all_ctx can't lock dev->mode_config.mutex due to locking inversion
> between that plain mutex and the ww dance.
I think that lock should die and all its callers should use connection_mutex instead. :-)
> - As a consequence we can only acquire ww mutexes. And we have a function
> which does most of that already: lock_all_crtc, and that even takes an
> acquire ctx! So my proposal would be to move the
> ww_mutex_lock(mode_config->connection_mutex) into that function too and
> rename it to lock_all_ctx. That nicely cleans up a bunch of callers too.
>
> The behind leaving out the ww backoff dance is that any caller who has
> an explicit acquire_ctx needs to have that backoff dance anyway. And if
> we hide it in lock_all_ctx this might result in driver-private ww
> mutexes getting silently dropped (since we only retry lock_all_ctx and
> not the top-level loop), leading to really subtle bugs. Atm there's no
> driver-private locks yet, but might very well happen.
I'm not sure there are benefits to extra driver-private locks. I've considered it for
protecting some of i915 state, but connection_mutex is always taken during
modeset, so that lock works. For other state normal mutexes were sufficient.
> - With that design for lock_all_ctx to only take ww mutexes there's no
> need for unlock_all_ctx - we already have the drm_modeset_drop_locks
> function for that.
Agreed. :-)
~Maarten
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
