On Mon, May 26, 2014 at 10:35 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Mon, May 26, 2014 at 07:56:50AM -0400, Rob Clark wrote:
>> On Mon, May 26, 2014 at 4:23 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> > On Sun, May 25, 2014 at 07:16:43PM -0400, Rob Clark wrote:
>> >> On Sun, May 25, 2014 at 6:10 PM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>> >> > On Sat, May 24, 2014 at 8:30 PM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>> >> >> @@ -8002,7 +8002,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
>> >> >> if (encoder->crtc) {
>> >> >> crtc = encoder->crtc;
>> >> >>
>> >> >> - mutex_lock(&crtc->mutex);
>> >> >> + drm_modeset_lock(&crtc->mutex, NULL);
>> >> >
>> >> >
>> >> > This is pretty much the reason why I think switching the
>> >> > mode_config.mutex to a ww_mutex is a bad idea: This call here nests
>> >> > within the mode_config.mutex and so must be acquired. Wiring the
>> >> > acquire context through everything is going to be fairly horrible,
>> >> > especially since you must be able to bail out when trying to lock with
>> >> > an axquire context.
>> >>
>> >> which is the call-path to here from mode_config.mutex? Is it possible
>> >> to just move the locking to a higher level for a
>> >> drm_modeset_lock_all()?
>> >
>> > Connector probing. And the entire point of crtc locks was to _not_ block
>> > all screen updates while we poke for a new edid or do load balancing. If
>> > you want to test this you need a gen3/4 with tv-out (native, not through
>> > sdvo) or a gen2 or i915g/gm with vga.
>>
>>
>> hmm, I guess I'm still not quite seeing the issue. For non-atomic
>> paths, we are grabbing mode_config and/or crtc mutex as bare mutexes
>> in same spots as we did before. So if it worked before without
>> nested_lock stuff it should still work now.
>
> Thread A is doing output probing.
>
> Thread B is doing atomic modeset
>
> Grabs mode_config.mutex
>
> Grabs crtc_A->ww_mutex
>
> Tries to grab crtc_A->ww_mutex,
> blocks since normal ww_mutex_lock
>
> Tries to grab mode_config.mutex with ww
> acuiquire context, blocks since current
> holder hasn't acquired the mutex with a ww
> ticket
hmm, ok, I had thought this case, thread B would get -EDEADLK because
lock was held, and not his acquire ctx. If that is not the case, then
I would propose this:
All places doing things the old way, must grab mode_config.mutex first
currently. And we use mode_config.mutex to protect
mode_config.acquire_ctx. So all the lower spots grabbing individual
crtc mutexes can safely use mode_config.acquire_ctx.
Then the only headache is propagating -EDEADLK up the call stack. If
we are lucky, the all already propagate -EINTR, etc.
BR,
-R
>
> -> Deadlock.
>
> You really can't mix lock nesting with w/w and lock nesting with a static
> hierarchy. It's all or nothing.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
