[PATCH] ftrace:fix check for return value of register_module_notifier

[PATCH] ftrace:fix check for return value of register_module_notifier

[tom.leiming]

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

return zero should be correct, so fix it.

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

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5b606f4..140699a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
 				  __stop_mcount_loc);
 
 	ret = register_module_notifier(&ftrace_module_nb);
-	if (!ret)
+	if (ret)
 		pr_warning("Failed to register trace ftrace module notifier\n");
 
 	return;
-- 
1.6.0.GIT

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Frederic Weisbecker]

On Sun, May 17, 2009 at 03:31:38PM +0800, tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> return zero should be correct, so fix it.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  kernel/trace/ftrace.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5b606f4..140699a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
>  				  __stop_mcount_loc);
>  
>  	ret = register_module_notifier(&ftrace_module_nb);
> -	if (!ret)
> +	if (ret)
>  		pr_warning("Failed to register trace ftrace module notifier\n");
>  
>  	return;


Indeed!

register_module_notifier returns blocking_notifier_chain_register():

 *
 *	Currently always returns zero.
 */
int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
		struct notifier_block *n)

For now it will never return anything else than 0 but we can keep
the warning in case it changes one day, as you did.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Li Zefan]

tom.leiming@gmail.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> return zero should be correct, so fix it.
> 

Nice catch.

We have those bogus warnings:

 $ dmesg | grep Failed
 Failed to register trace ftrace module notifier
 Failed to register trace events module notifier

Could you fix the same bug in event_trace_init()?

> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  kernel/trace/ftrace.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5b606f4..140699a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
>  				  __stop_mcount_loc);
>  
>  	ret = register_module_notifier(&ftrace_module_nb);
> -	if (!ret)
> +	if (ret)
>  		pr_warning("Failed to register trace ftrace module notifier\n");
>  
>  	return;

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Ming Lei]

2009/5/18 Li Zefan <lizf@cn.fujitsu.com>:
> tom.leiming@gmail.com wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> return zero should be correct, so fix it.
>>
>
> Nice catch.
>
> We have those bogus warnings:
>
>  $ dmesg | grep Failed
>  Failed to register trace ftrace module notifier
>  Failed to register trace events module notifier
>
> Could you fix the same bug in event_trace_init()?

OK, I'll submit it later.

>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>>  kernel/trace/ftrace.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 5b606f4..140699a 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
>>                                 __stop_mcount_loc);
>>
>>       ret = register_module_notifier(&ftrace_module_nb);
>> -     if (!ret)
>> +     if (ret)
>>               pr_warning("Failed to register trace ftrace module notifier\n");
>>
>>       return;
>



-- 
Lei Ming

[tip:tracing/core] tracing: fix check for return value of register_module_notifier

[tip-bot for Ming Lei]

Commit-ID:  24ed0c4bfc7d2d7507bb9d50f7f3bbdcd85d76dd
Gitweb:     http://git.kernel.org/tip/24ed0c4bfc7d2d7507bb9d50f7f3bbdcd85d76dd
Author:     Ming Lei <tom.leiming@gmail.com>
AuthorDate: Sun, 17 May 2009 15:31:38 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 18 May 2009 10:24:13 +0200

tracing: fix check for return value of register_module_notifier

return zero should be correct, so fix it.

[ Impact: eliminate incorrect syslog message ]

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Acked-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: rostedt@goodmis.org
LKML-Reference: <1242545498-7285-1-git-send-email-tom.leiming@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 kernel/trace/ftrace.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5b606f4..140699a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
 				  __stop_mcount_loc);
 
 	ret = register_module_notifier(&ftrace_module_nb);
-	if (!ret)
+	if (ret)
 		pr_warning("Failed to register trace ftrace module notifier\n");
 
 	return;

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Ming Lei]

>From d0d91950165f1d0a05ce8f68b333c6f0724b0007 Mon Sep 17 00:00:00 2001
From: Ming Lei <tom.leiming@gmail.com>
Date: Sun, 17 May 2009 15:26:28 +0800
Subject: [PATCH] ftrace:fix check for return value of register_module_notifier(v2)

return zero should be correct, so fix it.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com> 
---
 kernel/trace/ftrace.c       |    2 +-
 kernel/trace/trace_events.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5b606f4..140699a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
 				  __stop_mcount_loc);
 
 	ret = register_module_notifier(&ftrace_module_nb);
-	if (!ret)
+	if (ret)
 		pr_warning("Failed to register trace ftrace module notifier\n");
 
 	return;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0eec0c5..9e91c4a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1174,7 +1174,7 @@ static __init int event_trace_init(void)
 	}
 
 	ret = register_module_notifier(&trace_module_nb);
-	if (!ret)
+	if (ret)
 		pr_warning("Failed to register trace events module notifier\n");
 
 	return 0;
-- 
1.6.0.GIT



-- 
Lei Ming

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Frederic Weisbecker]

On Mon, May 18, 2009 at 08:39:57PM +0800, Ming Lei wrote:
> From d0d91950165f1d0a05ce8f68b333c6f0724b0007 Mon Sep 17 00:00:00 2001
> From: Ming Lei <tom.leiming@gmail.com>
> Date: Sun, 17 May 2009 15:26:28 +0800
> Subject: [PATCH] ftrace:fix check for return value of register_module_notifier(v2)
> 
> return zero should be correct, so fix it.
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Hi,


You are making a confusion between Signed-off-by: and
Reported-by:

Indeed Li Zefan suggested you to expand your patch to fix
another place. And that's what you did in this patch, that's fine.

But Signed-off-by: has a special meaning, please read the
Developer's Certificate of Origin 1.1 in Documentation/SubmittingPatches.

You shoudn't add someone else Signed-off-by if he/she didn't gave it to
you.

Instead, if you implement in a v2 patch a suggestion someone made while
reviewing your v1 patch, you can credit him in the changelog:

v2: change foo, as suggested by li Zefan

And if it's a new patch that addresses a new review, you can
just put a Reported-by: tag.


Your first patch have been applied in tip:/tracing/core topic.
So could you please send a new patch which addresses only
the event_trace_init() callsite?

You can add the

Reported-by: Li Zefan <lizf@cn.fujitsu.com>

and also my

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

in advance.

Thanks Ming!

Frederic.

> Acked-by: Frederic Weisbecker <fweisbec@gmail.com> 
> ---
>  kernel/trace/ftrace.c       |    2 +-
>  kernel/trace/trace_events.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5b606f4..140699a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -2826,7 +2826,7 @@ void __init ftrace_init(void)
>  				  __stop_mcount_loc);
>  
>  	ret = register_module_notifier(&ftrace_module_nb);
> -	if (!ret)
> +	if (ret)
>  		pr_warning("Failed to register trace ftrace module notifier\n");
>  
>  	return;
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 0eec0c5..9e91c4a 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1174,7 +1174,7 @@ static __init int event_trace_init(void)
>  	}
>  
>  	ret = register_module_notifier(&trace_module_nb);
> -	if (!ret)
> +	if (ret)
>  		pr_warning("Failed to register trace events module notifier\n");
>  
>  	return 0;
> -- 
> 1.6.0.GIT
> 
> 
> 
> -- 
> Lei Ming
> --
> 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/

Re: [PATCH] ftrace:fix check for return value of register_module_notifier

[Ming Lei]

This patch fixes another check in event_trace_init.
Thanks.

>From 121f44b1658c137b8404756fdbd97dcf1d0c58f9 Mon Sep 17 00:00:00 2001
From: Ming Lei <tom.leiming@gmail.com>
Date: Mon, 18 May 2009 23:04:46 +0800
Subject: [PATCH] ftrace:fix check for return value of register_module_notifier in event_trace_init

return zero should be correct, so fix it.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Reported-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/trace/trace_events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0eec0c5..9e91c4a 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1174,7 +1174,7 @@ static __init int event_trace_init(void)
 	}
 
 	ret = register_module_notifier(&trace_module_nb);
-	if (!ret)
+	if (ret)
 		pr_warning("Failed to register trace events module notifier\n");
 
 	return 0;
-- 
1.6.0.GIT


-- 
Lei Ming