From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753784AbcBCJtn (ORCPT <rfc822;w@1wt.eu>);
	Wed, 3 Feb 2016 04:49:43 -0500
Received: from mail-wm0-f67.google.com ([74.125.82.67]:33414 "EHLO
	mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753548AbcBCJtj (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 3 Feb 2016 04:49:39 -0500
Date: Wed, 3 Feb 2016 10:49:36 +0100
From: Ingo Molnar <mingo@kernel.org>
To: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: akpm@linux-foundation.org, krinkin.m.u@gmail.com, mingo@elte.hu,
        peterz@infradead.org, mm-commits@vger.kernel.org,
        linux-kernel@vger.kernel.org, Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Thomas Gleixner <tglx@linutronix.de>
Subject: Re: +
 kernel-locking-lockdepc-make-lockdep-initialize-itself-on-demand.patch added
 to -mm tree
Message-ID: <20160203094936.GA17305@gmail.com>
References: <56b11d2d.vVw1kB2la7Y+70xF%akpm@linux-foundation.org>
 <20160203074430.GA32652@gmail.com>
 <56B1CB7F.5070207@virtuozzo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <56B1CB7F.5070207@virtuozzo.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:

> 
> 
> On 02/03/2016 10:44 AM, Ingo Molnar wrote:
> 
> > Yuck, I don't really like this.
> > 
> > Lockdep initialization must happen early on, and it should happen in a well 
> > defined place, not be opportunistic (and relatively random) like this, making it 
> > dependent on config options and calling contexts.
> > 
> > Also, in addition to properly ordering UBSAN initialization, how about fixing the 
> > silent crash by adding a lockdep warning to that place instead of an auto-init? 
> > 
> > The warning will turn lockdep off safely and will generate an actionable kernel 
> > message and stackdump upon which the init ordering fix can be done.
> > 
> 
> Something like this already done for DEBUG_LOCKDEP=y (except it initializes lockdep instead of turning it off).
> 
> look_up_lock_class():
> ...
> 	#ifdef CONFIG_DEBUG_LOCKDEP
> 		/*
> 		 * If the architecture calls into lockdep before initializing
> 		 * the hashes then we'll warn about it later. (we cannot printk
> 		 * right now)
> 		 */
> 		if (unlikely(!lockdep_initialized)) {
> 			lockdep_init();
> 			lockdep_init_error = 1;
> 			lock_init_error = lock->name;
> 			save_stack_trace(&lockdep_init_trace);
> 		}
> 	#endif

well, this is different, as we still generate an error - so it's not a 'permanent 
solution' as the changelog says.

> Silent crash happens only in DEBUG_LOCKDEP=n && LOCKDEP=y combination.
> So, what about simply removing this #ifdef (and the other one in lockdep_info() )?

That's fine with me, as long as we also fix the init bug that triggers this code.

Thanks,

	Ingo