From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Airlie Subject: Re: [rfc] fix output poll execute lockdep Date: Tue, 10 Jan 2017 21:46:26 +1000 Message-ID: References: <20170110021232.15389-1-airlied@gmail.com> <20170110094958.7ce6vfpum5zod2ta@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1028733616==" Return-path: Received: from mail-vk0-x22a.google.com (mail-vk0-x22a.google.com [IPv6:2607:f8b0:400c:c05::22a]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7D3906E672 for ; Tue, 10 Jan 2017 11:46:28 +0000 (UTC) Received: by mail-vk0-x22a.google.com with SMTP id 137so358005221vkl.0 for ; Tue, 10 Jan 2017 03:46:28 -0800 (PST) In-Reply-To: <20170110094958.7ce6vfpum5zod2ta@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: Daniel Vetter Cc: dri-devel List-Id: dri-devel@lists.freedesktop.org --===============1028733616== Content-Type: multipart/alternative; boundary=94eb2c07cff8c88b060545bc0881 --94eb2c07cff8c88b060545bc0881 Content-Type: text/plain; charset=UTF-8 On 10 Jan. 2017 19:50, "Daniel Vetter" wrote: On Tue, Jan 10, 2017 at 12:12:30PM +1000, Dave Airlie wrote: > On runtime resume, nouveau can try and take the mode_config > mutex in the poll reenable, however a poll can race with this, > and take the mutex and get stuck waiting for the runtime to > finish completion. These two patches allow the driver to > get hooked in before the mutex is taken. > > I think radeon/amdgpu will also need similar patches to nouveau. > > I found this while trying to track down a runtime regression > on W541 laptop, I'm not sure I found what the reporter was seeing yet. Hm, we fixed this by doing the rpm stuff always within any of the big core locks. And I think that's the much more reasonable design, at least as soon as you have more fine-grained locking. But maybe there's a cheap way out - why does nouveau take the modeset lock? Ime you can remove a lot of the kms locking super easily because it's all cargo-culted. The last hold-out was connector_list walking, but that's fixed now and also doesn't need any of the heavy-weight kms locks anymore. Reenable polling takes the lock. Dave. Asking since if you really need kms locks in rpm paths, and the general design is to grab/drop rpm outside of everything (as nouveau does with the overall ioctl wrapper trick), then there's fundamentally a deadlock, or at least a nice inversion. It shouldn't really be needed ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch --94eb2c07cff8c88b060545bc0881 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On 10 Jan. 2017 19:50, "Daniel Vetter" <daniel@ffwll.ch> wrote:
On Tue, Jan 10, 2017= at 12:12:30PM +1000, Dave Airlie wrote:
> On runtime resume, nouveau can try and take the mode_config
> mutex in the poll reenable, however a poll can race with this,
> and take the mutex and get stuck waiting for the runtime to
> finish completion. These two patches allow the driver to
> get hooked in before the mutex is taken.
>
> I think radeon/amdgpu will also need similar patches to nouveau.
>
> I found this while trying to track down a runtime regression
> on W541 laptop, I'm not sure I found what the reporter was seeing = yet.

Hm, we fixed this by doing the rpm stuff always within any of the big= core
locks. And I think that's the much more reasonable design, at least as<= br> soon as you have more fine-grained locking.

But maybe there's a cheap way out - why does nouveau take the modeset lock? Ime you can remove a lot of the kms locking super easily because
it's all cargo-culted. The last hold-out was connector_list walking, bu= t
that's fixed now and also doesn't need any of the heavy-weight kms = locks
anymore.

Reenable polling takes the lock.=C2=A0

Dave.


Asking since if you really need kms locks in rpm paths, and the general
design is to grab/drop rpm outside of everything (as nouveau does with the<= br> overall ioctl wrapper trick), then there's fundamentally a deadlock, or= at
least a nice inversion. It shouldn't really be needed ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http:= //blog.ffwll.ch

--94eb2c07cff8c88b060545bc0881-- --===============1028733616== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1028733616==--