Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751283AbdH2S5c (ORCPT ); Tue, 29 Aug 2017 14:57:32 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:49231 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751186AbdH2S5a (ORCPT ); Tue, 29 Aug 2017 14:57:30 -0400 Date: Tue, 29 Aug 2017 20:57:27 +0200 From: Peter Zijlstra To: Byungchul Park Cc: Tejun Heo , Byungchul Park , johannes.berg@intel.com, Ingo Molnar , tglx@linutronix.de, "linux-kernel@vger.kernel.org" , kernel-team@lge.com Subject: Re: [RFC] workqueue: remove manual lockdep uses to detect deadlocks Message-ID: <20170829185727.GY32112@worktop.programming.kicks-ass.net> References: <1503650463-14582-1-git-send-email-byungchul.park@lge.com> <20170825133442.GU491396@devbig577.frc2.facebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1448 Lines: 50 On Sat, Aug 26, 2017 at 12:49:26AM +0900, Byungchul Park wrote: > > However, how would it distinguish things like flushing another work > > I think it must be distinguished with what it actually waits for, e.i. > completion > variables instead of work or wq. I will make it next week and let you know. So no. The existing annotations are strictly better than relying on cross-release. As you know the problem with cross-release is that it is timing dependent. You need to actually observe the problematic sequence before it can warn, and only the whole instance->class mapping saves us from actually hitting the deadlock. Cross-release can result in deadlocks without warnings. If you were to run: mutex_lock(A); mutex_lock(A); complete(C); wait_for_completion(C); You'd deadlock without issue. Only if we observe this: mutex_lock(A); wait_for_completion(C); mutex_lock(A); complete(C); Where we acquire A after wait_for_completion() but before complete() will we observe the deadlock. The same would be true for using cross-release for workqueues as well, something like: W: mutex_lock(A) mutex_lock(A) flush_work(W) would go unreported whereas the current workqueue annotation will generate a splat. This does not mean cross-release isn't worth it, its better than nothing, but its strictly weaker than traditional annotations. So where a traditional annotation is possible, we should use them.