From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965122AbdEOTGr (ORCPT ); Mon, 15 May 2017 15:06:47 -0400 Received: from mail.kernel.org ([198.145.29.99]:53590 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934078AbdEOTGq (ORCPT ); Mon, 15 May 2017 15:06:46 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0A4FE2397D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Mon, 15 May 2017 15:06:43 -0400 From: Steven Rostedt To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Mathieu Desnoyers , "Paul E. McKenney" , Masami Hiramatsu Subject: Re: [RFC][PATCH 0/5] perf/tracing/cpuhotplug: Fix locking order Message-ID: <20170515150643.72eafc46@gandalf.local.home> In-Reply-To: <20170512194956.GH4626@worktop.programming.kicks-ass.net> References: <20170512171544.100715273@goodmis.org> <20170512194956.GH4626@worktop.programming.kicks-ass.net> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 12 May 2017 21:49:56 +0200 Peter Zijlstra wrote: > In general we avoid nested locking in the kernel. Nested locking makes > an absolute mockery of locking rules and what all gets protected. I'm not against the goal of having get_online_cpus() not be nested, but I don't agree with the above comment. If we look at locking in a more abstract view, lock(A) is just something to protect critical section A. Lock(B) is just something to protect critical section B. To prevent deadlocks, if one enters critical section A and then enters critical section B before leaving critical section A, then the system must never enter critical section B and then enter critical section A. But for nested locks, the nested lock is not a new critical section. If we have lock(A); ...; lock(A); ...; unlock(A); ...; unlock(A); that just shows a single entry into critical section A. Even if we have: lock(A);...; lock(B); ...; lock(A); ...; unlock(A); ...; unlock(B); ...; unlock(A); As long as there never exits a lock(B); ...; lock(A); without first taking lock A before taking lock(B). Because the rules only matter when entering a critical section, and the taking of the second lock(A) is basically just a nop. We do this all the time in the kernel, but we just don't do it with nesting locks. We usually do it with __func()s. void func(void) { lock(A); __func(); unlock(A); } and later we could even have: void foo(void) { lock(A); ...; lock(B); ...; __func(); ...; unlock(B); ...; unlock(A); } If lock(A) was able to nest, then __func() wouldn't be needed. We could always just call func() which would take lock(A); lockdep will still work just fine, as it would only care when the lock is first taken, not its nesting. One thing we need to do with the current approach is void __func(void) { lockdep_assert_held(A); ...; } to make sure that A is held when calling __func(). Again, I'm not against the lofty goal of having no nesting of get_online_cpus(), but I just don't agree that nesting locks make a mockery out of the locking rules. -- Steve