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 at ffwll.ch> 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 at ffwll.ch> wrote: > >> > On Sat, May 24, 2014 at 8:30 PM, Rob Clark <robdclark at gmail.com> > >> > 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 -> 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