From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 03/17] drm: convert crtc and mode_config to ww_mutex Date: Mon, 26 May 2014 16:36:59 +0200 Message-ID: <20140526143659.GT14357@phenom.ffwll.local> References: <1400956226-28053-1-git-send-email-robdclark@gmail.com> <1400956226-28053-4-git-send-email-robdclark@gmail.com> <20140526082333.GW14357@phenom.ffwll.local> <20140526143504.GS14357@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f45.google.com (mail-wg0-f45.google.com [74.125.82.45]) by gabe.freedesktop.org (Postfix) with ESMTP id 1D6226E294 for ; Mon, 26 May 2014 07:37:04 -0700 (PDT) Received: by mail-wg0-f45.google.com with SMTP id m15so8209265wgh.16 for ; Mon, 26 May 2014 07:37:03 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140526143504.GS14357@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Clark Cc: dri-devel List-Id: dri-devel@lists.freedesktop.org On Mon, May 26, 2014 at 04:35:04PM +0200, Daniel Vetter 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 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 wrote: > > >> > On Sat, May 24, 2014 at 8:30 PM, Rob Clark 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 .. blocks since normal ww_mutex_lock without acquire ticket. Otherwise it's a bit unclear. -Daniel > > 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch