From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751920AbbJTOkY (ORCPT ); Tue, 20 Oct 2015 10:40:24 -0400 Received: from mail-qk0-f182.google.com ([209.85.220.182]:35463 "EHLO mail-qk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751735AbbJTOkU (ORCPT ); Tue, 20 Oct 2015 10:40:20 -0400 Date: Tue, 20 Oct 2015 10:40:12 -0400 From: Jerome Glisse To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, =?iso-8859-1?B?Suly9G1l?= Glisse , Ingo Molnar , Sasha Levin Subject: Re: [PATCH] locking/lockdep: Fix expected depth value in __lock_release() Message-ID: <20151020144011.GA4277@gmail.com> References: <1445283028-7865-1-git-send-email-j.glisse@gmail.com> <20151020121853.GD17308@twins.programming.kicks-ass.net> <20151020124218.GA3384@gmail.com> <20151020130729.GE17308@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20151020130729.GE17308@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 20, 2015 at 03:07:29PM +0200, Peter Zijlstra wrote: > On Tue, Oct 20, 2015 at 08:42:19AM -0400, Jerome Glisse wrote: > > On Tue, Oct 20, 2015 at 02:18:53PM +0200, Peter Zijlstra wrote: > > > > What code did you find that triggered this? That is, what code is taking > > > nested locks with other locks in the middle? (Not wrong per-se, just > > > curious how that would come about). > > > > Well i am not able to reproduce myself but it happens as part of > > mm_drop_all_locks() as to which lock does trigger i am unsure as > > all the i_mmap_rwsem are taken one after the other and same for > > anon_vma rwsem so they should already coalesce properly. My guess > > is that code calling all lock also have a mutex and once all vma > > lock are drop the mutex coalesce with mm_all_locks_mutex. > > Ah yes, mm_all_locks_mutex looks like a likely candidate. > > Curious, this code is ancient, and I've never seen a report of this > triggering. > > > > > This patch adjust the expect depth value by decrementing it if > > > > what was previously 2 entry inside the stack are coalesced into > > > > only one entry. > > > > > > Would it not make more sense to scan the entire hlock stack on > > > __lock_acquire() and avoid getting collapsible entries in the first > > > place? > > > > > > Something like so... > > > > It would work too, probably more compute intensive than my solution > > but this is lockdep code so i guess it is fine. Also dunno if we loose > > any valuable information by not keeping the stack ordered so one > > can check order in whick lock are taken. > > Right; its a little bit more expensive, but only for acquires with > nest_lock set, which should be rare. > > As to the order; since they're all of the same class, its fine to > collapse them. > > However the proposed alternative avoids 'strange' boundary cases like: > > mutex_lock(&top_lock); > > for (...) { > mutex_lock_nest_lock(&obj->lock1, &top_lock); > mutex_lock_nest_lock(&obj->lock2, &top_lock); > } > > Which would currently result in running our of lock stack space real > quick since it would never be able to collapse. > > In any case, can you 'test' the proposed alternative in any way? I will ask for it to be tested probably gonna take couple days before i hear back. I will report as soon as i have confirmation. Cheers, Jérôme