[PATCH 1/2] init/main.c: execute lockdep_init as early as possible

[PATCH 1/2] init/main.c: execute lockdep_init as early as possible

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

This patch removes the lockdep warning[1] on ARM platform.
The warning is caused by printk inside smp_setup_processor_id.

It is safe to do this because lockdep_init doesn't depend on
smp_setup_processor_id, so make printk can be called as early
as possible without lockdep complainment.

[1], lockdep warning
[    0.000000] WARNING: lockdep init error! Arch code didn't call
lockdep_init() early enough?
[    0.000000] Call stack leading to lockdep invocation was:
[    0.000000]  [<c00164bc>] save_stack_trace_tsk+0x0/0x90
[    0.000000]  [<ffffffff>] 0xffffffff

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 init/main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 217ed23..2c76efb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -469,13 +469,12 @@ asmlinkage void __init start_kernel(void)
 	char * command_line;
 	extern const struct kernel_param __start___param[], __stop___param[];
 
-	smp_setup_processor_id();
-
 	/*
 	 * Need to run as early as possible, to initialize the
 	 * lockdep hash:
 	 */
 	lockdep_init();
+	smp_setup_processor_id();
 	debug_objects_early_init();
 
 	/*
-- 
1.7.5.4

[PATCH 2/2] lockdep: print lock name for lockdep_init_error

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

This patch prints the name of the lock which is acquired
before lockdep_init is called, so that user can easily find
which lock trigged the lockdep init error warning.

This patch also removes the lockdep_init_error message
of "Arch code didn't call lockdep_init() early enough?" since
lockdep_init is called in arch independent code now.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 kernel/lockdep.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 4e7e672..8141317 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -430,6 +430,7 @@ unsigned int max_lockdep_depth;
  * about it later on, in lockdep_info().
  */
 static int lockdep_init_error;
+static const char *lock_init_error;
 static unsigned long lockdep_init_trace_data[20];
 static struct stack_trace lockdep_init_trace = {
 	.max_entries = ARRAY_SIZE(lockdep_init_trace_data),
@@ -651,6 +652,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
 	if (unlikely(!lockdep_initialized)) {
 		lockdep_init();
 		lockdep_init_error = 1;
+		lock_init_error = lock->name;
 		save_stack_trace(&lockdep_init_trace);
 	}
 #endif
@@ -3965,7 +3967,8 @@ void __init lockdep_info(void)
 
 #ifdef CONFIG_DEBUG_LOCKDEP
 	if (lockdep_init_error) {
-		printk("WARNING: lockdep init error! Arch code didn't call lockdep_init() early enough?\n");
+		printk("WARNING: lockdep init error! lock-%s was acquired"
+			"before lockdep_init\n", lock_init_error);
 		printk("Call stack leading to lockdep invocation was:\n");
 		print_stack_trace(&lockdep_init_trace, 0);
 	}
-- 
1.7.5.4

Re: [PATCH 2/2] lockdep: print lock name for lockdep_init_error

[Yong Zhang]

On Thu, Nov 17, 2011 at 01:34:32PM +0800, tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch prints the name of the lock which is acquired
> before lockdep_init is called, so that user can easily find
> which lock trigged the lockdep init error warning.

Should we care about that?

I think lockdep_init() called early enough give more hint.
Such as be the first C function called by arch.

Thanks,
Yong

> 
> This patch also removes the lockdep_init_error message
> of "Arch code didn't call lockdep_init() early enough?" since
> lockdep_init is called in arch independent code now.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  kernel/lockdep.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index 4e7e672..8141317 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -430,6 +430,7 @@ unsigned int max_lockdep_depth;
>   * about it later on, in lockdep_info().
>   */
>  static int lockdep_init_error;
> +static const char *lock_init_error;
>  static unsigned long lockdep_init_trace_data[20];
>  static struct stack_trace lockdep_init_trace = {
>  	.max_entries = ARRAY_SIZE(lockdep_init_trace_data),
> @@ -651,6 +652,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
>  	if (unlikely(!lockdep_initialized)) {
>  		lockdep_init();
>  		lockdep_init_error = 1;
> +		lock_init_error = lock->name;
>  		save_stack_trace(&lockdep_init_trace);
>  	}
>  #endif
> @@ -3965,7 +3967,8 @@ void __init lockdep_info(void)
>  
>  #ifdef CONFIG_DEBUG_LOCKDEP
>  	if (lockdep_init_error) {
> -		printk("WARNING: lockdep init error! Arch code didn't call lockdep_init() early enough?\n");
> +		printk("WARNING: lockdep init error! lock-%s was acquired"
> +			"before lockdep_init\n", lock_init_error);
>  		printk("Call stack leading to lockdep invocation was:\n");
>  		print_stack_trace(&lockdep_init_trace, 0);
>  	}
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Only stand for myself

Re: [PATCH 2/2] lockdep: print lock name for lockdep_init_error

[Ming Lei]

Hi,

Thanks for your review.

On Thu, Nov 17, 2011 at 2:02 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> On Thu, Nov 17, 2011 at 01:34:32PM +0800, tom.leiming@gmail.com wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> This patch prints the name of the lock which is acquired
>> before lockdep_init is called, so that user can easily find
>> which lock trigged the lockdep init error warning.
>
> Should we care about that?

Yes, we should, see below.

>
> I think lockdep_init() called early enough give more hint.
> Such as be the first C function called by arch.

Maybe not early enough, before start_kernel is executed, arch
asm code still can call some C function, can't it?

The patch is just an improvement on original lockdep_init_error
detect, plus fix on incorrect debug message.

thanks,
-- 
Ming Lei

Re: [PATCH 2/2] lockdep: print lock name for lockdep_init_error

[Yong Zhang]

On Thu, Nov 17, 2011 at 04:01:16PM +0800, Ming Lei wrote:
> Hi,
> 
> Thanks for your review.
> 
> On Thu, Nov 17, 2011 at 2:02 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> > On Thu, Nov 17, 2011 at 01:34:32PM +0800, tom.leiming@gmail.com wrote:
> >> From: Ming Lei <tom.leiming@gmail.com>
> >>
> >> This patch prints the name of the lock which is acquired
> >> before lockdep_init is called, so that user can easily find
> >> which lock trigged the lockdep init error warning.
> >
> > Should we care about that?
> 
> Yes, we should, see below.
> 
> >
> > I think lockdep_init() called early enough give more hint.
> > Such as be the first C function called by arch.
> 
> Maybe not early enough, before start_kernel is executed, arch
> asm code still can call some C function, can't it?

To me that still means 'Arch code didn't call lockdep_init()
early enough'.

But anyway, let Peter/Ingo decide it.

Thanks,
Yong

Re: [PATCH 2/2] lockdep: print lock name for lockdep_init_error

[Peter Zijlstra]

On Thu, 2011-11-17 at 16:44 +0800, Yong Zhang wrote:
> On Thu, Nov 17, 2011 at 04:01:16PM +0800, Ming Lei wrote:

> To me that still means 'Arch code didn't call lockdep_init()
> early enough'.
> 
> But anyway, let Peter/Ingo decide it.

I kinda like the hint as to what might have ran too early, so I've
queued it.

Re: [PATCH 2/2] lockdep: print lock name for lockdep_init_error

[Ryan Mallon]

On 17/11/11 16:34, tom.leiming@gmail.com wrote:

> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch prints the name of the lock which is acquired
> before lockdep_init is called, so that user can easily find
> which lock trigged the lockdep init error warning.
> 
> This patch also removes the lockdep_init_error message
> of "Arch code didn't call lockdep_init() early enough?" since
> lockdep_init is called in arch independent code now.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  kernel/lockdep.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index 4e7e672..8141317 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -430,6 +430,7 @@ unsigned int max_lockdep_depth;
>   * about it later on, in lockdep_info().
>   */
>  static int lockdep_init_error;
> +static const char *lock_init_error;


You can now remove lockdep_init_error and just test against
lock_init_error below instead.

~Ryan

>  static unsigned long lockdep_init_trace_data[20];
>  static struct stack_trace lockdep_init_trace = {
>  	.max_entries = ARRAY_SIZE(lockdep_init_trace_data),
> @@ -651,6 +652,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
>  	if (unlikely(!lockdep_initialized)) {
>  		lockdep_init();
>  		lockdep_init_error = 1;
> +		lock_init_error = lock->name;
>  		save_stack_trace(&lockdep_init_trace);
>  	}
>  #endif
> @@ -3965,7 +3967,8 @@ void __init lockdep_info(void)
>  
>  #ifdef CONFIG_DEBUG_LOCKDEP
>  	if (lockdep_init_error) {
> -		printk("WARNING: lockdep init error! Arch code didn't call lockdep_init() early enough?\n");
> +		printk("WARNING: lockdep init error! lock-%s was acquired"
> +			"before lockdep_init\n", lock_init_error);
>  		printk("Call stack leading to lockdep invocation was:\n");
>  		print_stack_trace(&lockdep_init_trace, 0);
>  	}

[tip:core/locking] lockdep: Print lock name in lockdep_init_error()

[tip-bot for Ming Lei]

Commit-ID:  81140acc66322dcde8346dabdf1ab4c229fce8d4
Gitweb:     http://git.kernel.org/tip/81140acc66322dcde8346dabdf1ab4c229fce8d4
Author:     Ming Lei <tom.leiming@gmail.com>
AuthorDate: Thu, 17 Nov 2011 13:34:32 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 08:16:55 +0100

lockdep: Print lock name in lockdep_init_error()

This patch prints the name of the lock which is acquired
before lockdep_init() is called, so that users can easily
find which lock triggered the lockdep init error warning.

This patch also removes the lockdep_init_error() message
of "Arch code didn't call lockdep_init() early enough?"
since lockdep_init() is called in arch independent code now.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1321508072-23853-2-git-send-email-tom.leiming@gmail.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/lockdep.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 54cc0dc..e69d633 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -431,6 +431,7 @@ unsigned int max_lockdep_depth;
  * about it later on, in lockdep_info().
  */
 static int lockdep_init_error;
+static const char *lock_init_error;
 static unsigned long lockdep_init_trace_data[20];
 static struct stack_trace lockdep_init_trace = {
 	.max_entries = ARRAY_SIZE(lockdep_init_trace_data),
@@ -657,6 +658,7 @@ look_up_lock_class(struct lockdep_map *lock, unsigned int subclass)
 	if (unlikely(!lockdep_initialized)) {
 		lockdep_init();
 		lockdep_init_error = 1;
+		lock_init_error = lock->name;
 		save_stack_trace(&lockdep_init_trace);
 	}
 #endif
@@ -3978,7 +3980,8 @@ void __init lockdep_info(void)
 
 #ifdef CONFIG_DEBUG_LOCKDEP
 	if (lockdep_init_error) {
-		printk("WARNING: lockdep init error! Arch code didn't call lockdep_init() early enough?\n");
+		printk("WARNING: lockdep init error! lock-%s was acquired"
+			"before lockdep_init\n", lock_init_error);
 		printk("Call stack leading to lockdep invocation was:\n");
 		print_stack_trace(&lockdep_init_trace, 0);
 	}

Re: [PATCH 1/2] init/main.c: execute lockdep_init as early as possible

[Yong Zhang]

On Thu, Nov 17, 2011 at 01:34:31PM +0800, tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch removes the lockdep warning[1] on ARM platform.
> The warning is caused by printk inside smp_setup_processor_id.
> 
> It is safe to do this because lockdep_init doesn't depend on
> smp_setup_processor_id, so make printk can be called as early
> as possible without lockdep complainment.
> 
> [1], lockdep warning
> [    0.000000] WARNING: lockdep init error! Arch code didn't call
> lockdep_init() early enough?
> [    0.000000] Call stack leading to lockdep invocation was:
> [    0.000000]  [<c00164bc>] save_stack_trace_tsk+0x0/0x90
> [    0.000000]  [<ffffffff>] 0xffffffff
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

Seems start_kernel() is the first C function called in arm.
IOW, this may be the simplest way for your issue.

Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>

> ---
>  init/main.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index 217ed23..2c76efb 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -469,13 +469,12 @@ asmlinkage void __init start_kernel(void)
>  	char * command_line;
>  	extern const struct kernel_param __start___param[], __stop___param[];
>  
> -	smp_setup_processor_id();
> -
>  	/*
>  	 * Need to run as early as possible, to initialize the
>  	 * lockdep hash:
>  	 */
>  	lockdep_init();
> +	smp_setup_processor_id();
>  	debug_objects_early_init();
>  
>  	/*
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Only stand for myself

[tip:core/locking] init/main.c: Execute lockdep_init() as early as possible

[tip-bot for Ming Lei]

Commit-ID:  73839c5b2eacc15cb0aa79c69b285fc659fa8851
Gitweb:     http://git.kernel.org/tip/73839c5b2eacc15cb0aa79c69b285fc659fa8851
Author:     Ming Lei <tom.leiming@gmail.com>
AuthorDate: Thu, 17 Nov 2011 13:34:31 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 08:16:53 +0100

init/main.c: Execute lockdep_init() as early as possible

This patch fixes a lockdep warning on ARM platforms:

  [    0.000000] WARNING: lockdep init error! Arch code didn't call lockdep_init() early enough?
  [    0.000000] Call stack leading to lockdep invocation was:
  [    0.000000]  [<c00164bc>] save_stack_trace_tsk+0x0/0x90
  [    0.000000]  [<ffffffff>] 0xffffffff

The warning is caused by printk inside smp_setup_processor_id().

It is safe to do this because lockdep_init() doesn't depend on
smp_setup_processor_id(), so improve things that printk can be
called as early as possible without lockdep complaint.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/1321508072-23853-1-git-send-email-tom.leiming@gmail.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 init/main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 217ed23..2c76efb 100644
--- a/init/main.c
+++ b/init/main.c
@@ -469,13 +469,12 @@ asmlinkage void __init start_kernel(void)
 	char * command_line;
 	extern const struct kernel_param __start___param[], __stop___param[];
 
-	smp_setup_processor_id();
-
 	/*
 	 * Need to run as early as possible, to initialize the
 	 * lockdep hash:
 	 */
 	lockdep_init();
+	smp_setup_processor_id();
 	debug_objects_early_init();
 
 	/*