From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933261AbbHKBcR (ORCPT ); Mon, 10 Aug 2015 21:32:17 -0400 Received: from mail-ob0-f170.google.com ([209.85.214.170]:33415 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932740AbbHKBcQ (ORCPT ); Mon, 10 Aug 2015 21:32:16 -0400 Date: Tue, 11 Aug 2015 09:32:04 +0800 From: Boqun Feng To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org Subject: Re: [Question] lockdep: Is nested lock handled correctly? Message-ID: <20150811013204.GC4606@fixme-laptop.cn.ibm.com> References: <20150810095247.GA4606@fixme-laptop.cn.ibm.com> <20150810114228.GY16853@twins.programming.kicks-ass.net> <20150810134924.GB4606@fixme-laptop.cn.ibm.com> <20150810142417.GC16853@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150810142417.GC16853@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, On Mon, Aug 10, 2015 at 04:24:17PM +0200, Peter Zijlstra wrote: > On Mon, Aug 10, 2015 at 09:49:24PM +0800, Boqun Feng wrote: > > Though I don't want to have a locking order like that either, we can't > > stop others from using that order(maybe a good design review will) and > > lockdep yells something -unrelated- in such an order. > > > > I think we can either let lockdep complain if some one uses this > > locking order or clean up current code a little bit to tolarent this. > > > > If you really think we should do something about it, I can write the > > patch and add test cases. > > > Maybe something like the below in __lock_acquire(): > > /* Daft bugger, can't guard a nesting order with the same lock class */ > if (DEBUG_LOCKS_WARN_ON(lock == nest_lock)) > return 0; > > ? I may not understand this well.. but I think this may not detect the problem. The problem is: A correct nesting order get disturbed by other locks acquired before the nested lock acquired and release before the nested, which makes two held_lock structures merged during __lock_acquire(). I think we can detect this in __lock_release(): diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 8acfbf7..e75f622 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3427,6 +3427,19 @@ found_it: curr->lockdep_depth = i; curr->curr_chain_key = hlock->prev_chain_key; + /* + * We are going to "reacquire" the rest of stack, but we find out + * __lock_acquire() will merge the next hlock into prev_hlock, + * which means this is not a good time to release this lock and lock + * users might need to reconsider the locking design. + */ + if (prev_hlock && (i+1) < depth) { + hlock = curr->held_locks + i + 1; + if (DEBUG_LOCKS_WARN_ON(hlock->nest_lock && + hlock->class_idx == prev_hlock->class_idx)) + return 0; + } + for (i++; i < depth; i++) { hlock = curr->held_locks + i; if (!__lock_acquire(hlock->instance, Regards, Boqun