Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
Hi Luck, Borislav, OK, since you all think it is not necessary, I think I will drop patch1. And thanks for your comments. :) So, how about patch2 ? If you need more detail, please tell me. Thanks. :) On 10/24/2012 12:16 AM, Luck, Tony wrote: First of all, I do think I was answering your question. As I said before, if an online cpu == dying here, there must be something wrong. Am I right here ? Yes - but there is a fuzzy line over where it is good to check for "something wrong" or whether to trust that the caller of the function knew what they were doing. For example we trust that "dying" is a valid cpu number. If we were super-paranoid that someone might change the code and call us with a bad argument, we might add: BUG_ON(dying< 0 || dying>= MAX_NR_CPUS); This would certainly help debug the case if someone did make a bogus change ... but I think it is clear that this test is way past the fuzzy line and into pointless. Back to the case in question: do we think there is a credible case where the "dying" cpu can show up in our "for_each_cpu_online()" loop? The original author of the code was worried enough to make a test, but thought that the appropriate action was to silently skip it. You want to add a WARN_ON, which will cause users who read the console logs to worry, but that most users will never see. -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
> First of all, I do think I was answering your question. As I said > before, if an online cpu == dying here, there must be something wrong. > Am I right here ? Yes - but there is a fuzzy line over where it is good to check for "something wrong" or whether to trust that the caller of the function knew what they were doing. For example we trust that "dying" is a valid cpu number. If we were super-paranoid that someone might change the code and call us with a bad argument, we might add: BUG_ON(dying < 0 || dying >= MAX_NR_CPUS); This would certainly help debug the case if someone did make a bogus change ... but I think it is clear that this test is way past the fuzzy line and into pointless. Back to the case in question: do we think there is a credible case where the "dying" cpu can show up in our "for_each_cpu_online()" loop? The original author of the code was worried enough to make a test, but thought that the appropriate action was to silently skip it. You want to add a WARN_ON, which will cause users who read the console logs to worry, but that most users will never see. -Tony
Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 07:30:21PM +0800, Tang Chen wrote: > First of all, I do think I was answering your question. As I said > before, if an online cpu == dying here, there must be something wrong. > Am I right here ? Please read the code. We're skipping the cpu == dying case. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 06:34:33PM +0800, Miao Xie wrote: > So we add this WARN_ON_ONCE(), it can tell the developers that there > is something wrong in the code if it is triggered. First of all, the WARN_ON_ONCE will fire only once during system lifetime (well, doh, of course) which diminishes debuggability significantly and then, the only other place which deals with CPU_POST_DEAD is kernel/stop_machine.c:cpu_stop_cpu_callback. So, just to sum up and finish this fruitless discussion: cmci_rediscover() correctly ignores the dying cpu and there's *absolutely* no need to warn. If you still think there is, you have to come up with a concrete example and a way for others to reproduce it. Then we can talk. End of story. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/23/2012 05:52 PM, Borislav Petkov wrote:
On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Let me ask you again, but I want you to think real hard this time:
"Why do we need to warn? What good would that bring us?"
Hi,
First of all, I do think I was answering your question. As I said
before, if an online cpu == dying here, there must be something wrong.
Am I right here ?
If so, I think the "good" is obvious. If we don't output anything when
an online cpu == dying, nobody will know this happens. The kernel is in
wrong state, but nobody knows that, I don't see any good.
Actually, I used BUG_ON() in my v1 patch. So I dropped the if statement.
But Tejun asked me to use WARN_ON_ONCE(). And I forgot to add the if
statement.
Please refer to https://lkml.org/lkml/2012/10/16/528
And again, the "good" is inform user the kernel is in wrong state.
Thanks. :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On tue, 23 Oct 2012 12:20:08 +0200, Borislav Petkov wrote: > On Tue, Oct 23, 2012 at 06:17:31PM +0800, Miao Xie wrote: >> This function is called after a cpu is offline, in other words, it is >> impossible that the cpu is still in cpu_online_mask. otherwise there >> is something wrong in the code. > > And? So we add this WARN_ON_ONCE(), it can tell the developers that there is something wrong in the code if it is triggered. Thanks Miao -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 06:17:31PM +0800, Miao Xie wrote: > This function is called after a cpu is offline, in other words, it is > impossible that the cpu is still in cpu_online_mask. otherwise there > is something wrong in the code. And? Are you answering my question or explaining the code just for the fun of it? -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On tue, 23 Oct 2012 11:52:34 +0200, Borislav Petkov wrote:
> On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
>> So, how about warn once, and continue:
>> if (cpu == dying) {
>> WARN_ON_ONCE(cpu == dying);
>> continue;
>> }
>>
>> or, use BUG_ON() instead ?
>
> Let me ask you again, but I want you to think real hard this time:
>
> "Why do we need to warn? What good would that bring us?"
>
This function is called after a cpu is offline, in other words, it is
impossible that the cpu is still in cpu_online_mask. otherwise there is
something wrong in the code.
Thanks
Miao
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
> So, how about warn once, and continue:
> if (cpu == dying) {
> WARN_ON_ONCE(cpu == dying);
> continue;
> }
>
> or, use BUG_ON() instead ?
Let me ask you again, but I want you to think real hard this time:
"Why do we need to warn? What good would that bring us?"
--
Regards/Gruss,
Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Let me ask you again, but I want you to think real hard this time:
Why do we need to warn? What good would that bring us?
--
Regards/Gruss,
Boris.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On tue, 23 Oct 2012 11:52:34 +0200, Borislav Petkov wrote:
On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Let me ask you again, but I want you to think real hard this time:
Why do we need to warn? What good would that bring us?
This function is called after a cpu is offline, in other words, it is
impossible that the cpu is still in cpu_online_mask. otherwise there is
something wrong in the code.
Thanks
Miao
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 06:17:31PM +0800, Miao Xie wrote: This function is called after a cpu is offline, in other words, it is impossible that the cpu is still in cpu_online_mask. otherwise there is something wrong in the code. And? Are you answering my question or explaining the code just for the fun of it? -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On tue, 23 Oct 2012 12:20:08 +0200, Borislav Petkov wrote: On Tue, Oct 23, 2012 at 06:17:31PM +0800, Miao Xie wrote: This function is called after a cpu is offline, in other words, it is impossible that the cpu is still in cpu_online_mask. otherwise there is something wrong in the code. And? So we add this WARN_ON_ONCE(), it can tell the developers that there is something wrong in the code if it is triggered. Thanks Miao -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/23/2012 05:52 PM, Borislav Petkov wrote:
On Tue, Oct 23, 2012 at 10:55:13AM +0800, Tang Chen wrote:
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Let me ask you again, but I want you to think real hard this time:
Why do we need to warn? What good would that bring us?
Hi,
First of all, I do think I was answering your question. As I said
before, if an online cpu == dying here, there must be something wrong.
Am I right here ?
If so, I think the good is obvious. If we don't output anything when
an online cpu == dying, nobody will know this happens. The kernel is in
wrong state, but nobody knows that, I don't see any good.
Actually, I used BUG_ON() in my v1 patch. So I dropped the if statement.
But Tejun asked me to use WARN_ON_ONCE(). And I forgot to add the if
statement.
Please refer to https://lkml.org/lkml/2012/10/16/528
And again, the good is inform user the kernel is in wrong state.
Thanks. :)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 06:34:33PM +0800, Miao Xie wrote: So we add this WARN_ON_ONCE(), it can tell the developers that there is something wrong in the code if it is triggered. First of all, the WARN_ON_ONCE will fire only once during system lifetime (well, doh, of course) which diminishes debuggability significantly and then, the only other place which deals with CPU_POST_DEAD is kernel/stop_machine.c:cpu_stop_cpu_callback. So, just to sum up and finish this fruitless discussion: cmci_rediscover() correctly ignores the dying cpu and there's *absolutely* no need to warn. If you still think there is, you have to come up with a concrete example and a way for others to reproduce it. Then we can talk. End of story. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Tue, Oct 23, 2012 at 07:30:21PM +0800, Tang Chen wrote: First of all, I do think I was answering your question. As I said before, if an online cpu == dying here, there must be something wrong. Am I right here ? Please read the code. We're skipping the cpu == dying case. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
First of all, I do think I was answering your question. As I said before, if an online cpu == dying here, there must be something wrong. Am I right here ? Yes - but there is a fuzzy line over where it is good to check for something wrong or whether to trust that the caller of the function knew what they were doing. For example we trust that dying is a valid cpu number. If we were super-paranoid that someone might change the code and call us with a bad argument, we might add: BUG_ON(dying 0 || dying = MAX_NR_CPUS); This would certainly help debug the case if someone did make a bogus change ... but I think it is clear that this test is way past the fuzzy line and into pointless. Back to the case in question: do we think there is a credible case where the dying cpu can show up in our for_each_cpu_online() loop? The original author of the code was worried enough to make a test, but thought that the appropriate action was to silently skip it. You want to add a WARN_ON, which will cause users who read the console logs to worry, but that most users will never see. -Tony
Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
Hi Luck, Borislav, OK, since you all think it is not necessary, I think I will drop patch1. And thanks for your comments. :) So, how about patch2 ? If you need more detail, please tell me. Thanks. :) On 10/24/2012 12:16 AM, Luck, Tony wrote: First of all, I do think I was answering your question. As I said before, if an online cpu == dying here, there must be something wrong. Am I right here ? Yes - but there is a fuzzy line over where it is good to check for something wrong or whether to trust that the caller of the function knew what they were doing. For example we trust that dying is a valid cpu number. If we were super-paranoid that someone might change the code and call us with a bad argument, we might add: BUG_ON(dying 0 || dying= MAX_NR_CPUS); This would certainly help debug the case if someone did make a bogus change ... but I think it is clear that this test is way past the fuzzy line and into pointless. Back to the case in question: do we think there is a credible case where the dying cpu can show up in our for_each_cpu_online() loop? The original author of the code was worried enough to make a test, but thought that the appropriate action was to silently skip it. You want to add a WARN_ON, which will cause users who read the console logs to worry, but that most users will never see. -Tony -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/22/2012 06:14 PM, Borislav Petkov wrote:
On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote:
I don't why before we just jumped over it. But I think if we have an
online cpu == dying here, it must be wrong. So I think we should warn
it, not just jump over it.
Why do we need to warn? What good would that bring us?
AFAICT, the check in cmci_rediscover is there to make sure we absolutely
don't rediscover on the dying cpu. I think it is a safety precaution in
concurrency scenarios between cpu hotplug and mce code.
Well, I see. I dropped the if statement. :)
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/22/2012 06:14 PM, Borislav Petkov wrote: On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote: I don't why before we just jumped over it. But I think if we have an online cpu == dying here, it must be wrong. So I think we should warn it, not just jump over it. Why do we need to warn? What good would that bring us? AFAICT, the check in cmci_rediscover is there to make sure we absolutely don't rediscover on the dying cpu. I think it is a safety precaution in concurrency scenarios between cpu hotplug and mce code. Hi Borislav, As I said, cmci_rediscover() is only used to handle CPU_POST_DEAD event. And is uses for_each_online_cpu to iterate all online cpus. If we get a online cpu == dying, I think we are in a wrong situation. I think at least, we should give a warning. Thanks. :) Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote: > I don't why before we just jumped over it. But I think if we have an > online cpu == dying here, it must be wrong. So I think we should warn > it, not just jump over it. Why do we need to warn? What good would that bring us? AFAICT, the check in cmci_rediscover is there to make sure we absolutely don't rediscover on the dying cpu. I think it is a safety precaution in concurrency scenarios between cpu hotplug and mce code. Thanks. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote: I don't why before we just jumped over it. But I think if we have an online cpu == dying here, it must be wrong. So I think we should warn it, not just jump over it. Why do we need to warn? What good would that bring us? AFAICT, the check in cmci_rediscover is there to make sure we absolutely don't rediscover on the dying cpu. I think it is a safety precaution in concurrency scenarios between cpu hotplug and mce code. Thanks. -- Regards/Gruss, Boris. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/22/2012 06:14 PM, Borislav Petkov wrote: On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote: I don't why before we just jumped over it. But I think if we have an online cpu == dying here, it must be wrong. So I think we should warn it, not just jump over it. Why do we need to warn? What good would that bring us? AFAICT, the check in cmci_rediscover is there to make sure we absolutely don't rediscover on the dying cpu. I think it is a safety precaution in concurrency scenarios between cpu hotplug and mce code. Hi Borislav, As I said, cmci_rediscover() is only used to handle CPU_POST_DEAD event. And is uses for_each_online_cpu to iterate all online cpus. If we get a online cpu == dying, I think we are in a wrong situation. I think at least, we should give a warning. Thanks. :) Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/22/2012 06:14 PM, Borislav Petkov wrote:
On Mon, Oct 22, 2012 at 10:10:24AM +0800, Tang Chen wrote:
I don't why before we just jumped over it. But I think if we have an
online cpu == dying here, it must be wrong. So I think we should warn
it, not just jump over it.
Why do we need to warn? What good would that bring us?
AFAICT, the check in cmci_rediscover is there to make sure we absolutely
don't rediscover on the dying cpu. I think it is a safety precaution in
concurrency scenarios between cpu hotplug and mce code.
Well, I see. I dropped the if statement. :)
So, how about warn once, and continue:
if (cpu == dying) {
WARN_ON_ONCE(cpu == dying);
continue;
}
or, use BUG_ON() instead ?
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/20/2012 12:40 AM, Borislav Petkov wrote:
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote:
cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
which means the corresponding cpu has already dead. As a result, it
won't be accessed in the for_each_online_cpu loop.
So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
Signed-off-by: Tang Chen
---
arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 38e49bc..481d152 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
cpumask_copy(old,>cpus_allowed);
for_each_online_cpu(cpu) {
- if (cpu == dying)
- continue;
+ WARN_ON_ONCE(cpu == dying);
Ok, I don't understand that:
we want to warn that the rediscovering is happening on a dying cpu?? And
before that, we simply jumped over it and didn't do the rediscovering
there? Why should we warn at all?
Hi Borislav,
As far as I know, cmci_rediscover() is only called in
mce_cpu_callback() to handle CPU_POST_DEAD event.
2362 if (action == CPU_POST_DEAD) {
2363 /* intentionally ignoring frozen here */
2364 cmci_rediscover(cpu);
2365 }
I didn't find anywhere else using this function. So I think the cpu
should be dead already when this function is called.
I don't why before we just jumped over it. But I think if we have an
online cpu == dying here, it must be wrong. So I think we should warn
it, not just jump over it.
Thanks. :)
Huh?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On 10/20/2012 12:40 AM, Borislav Petkov wrote:
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote:
cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
which means the corresponding cpu has already dead. As a result, it
won't be accessed in the for_each_online_cpu loop.
So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
Signed-off-by: Tang Chentangc...@cn.fujitsu.com
---
arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 38e49bc..481d152 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
cpumask_copy(old,current-cpus_allowed);
for_each_online_cpu(cpu) {
- if (cpu == dying)
- continue;
+ WARN_ON_ONCE(cpu == dying);
Ok, I don't understand that:
we want to warn that the rediscovering is happening on a dying cpu?? And
before that, we simply jumped over it and didn't do the rediscovering
there? Why should we warn at all?
Hi Borislav,
As far as I know, cmci_rediscover() is only called in
mce_cpu_callback() to handle CPU_POST_DEAD event.
2362 if (action == CPU_POST_DEAD) {
2363 /* intentionally ignoring frozen here */
2364 cmci_rediscover(cpu);
2365 }
I didn't find anywhere else using this function. So I think the cpu
should be dead already when this function is called.
I don't why before we just jumped over it. But I think if we have an
online cpu == dying here, it must be wrong. So I think we should warn
it, not just jump over it.
Thanks. :)
Huh?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote:
> cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
> which means the corresponding cpu has already dead. As a result, it
> won't be accessed in the for_each_online_cpu loop.
> So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
>
> Signed-off-by: Tang Chen
> ---
> arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
> b/arch/x86/kernel/cpu/mcheck/mce_intel.c
> index 38e49bc..481d152 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
> @@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
> cpumask_copy(old, >cpus_allowed);
>
> for_each_online_cpu(cpu) {
> - if (cpu == dying)
> - continue;
> + WARN_ON_ONCE(cpu == dying);
Ok, I don't understand that:
we want to warn that the rediscovering is happening on a dying cpu?? And
before that, we simply jumped over it and didn't do the rediscovering
there? Why should we warn at all?
Huh?
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote: > cmci_rediscover() is only called by the CPU_POST_DEAD event handler, > which means the corresponding cpu has already dead. As a result, it > won't be accessed in the for_each_online_cpu loop. > So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE(). > > Signed-off-by: Tang Chen > --- > arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote: cmci_rediscover() is only called by the CPU_POST_DEAD event handler, which means the corresponding cpu has already dead. As a result, it won't be accessed in the for_each_online_cpu loop. So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE(). Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) formletter This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly. /formletter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@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 v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
On Fri, Oct 19, 2012 at 01:45:27PM +0800, Tang Chen wrote:
cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
which means the corresponding cpu has already dead. As a result, it
won't be accessed in the for_each_online_cpu loop.
So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
---
arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 38e49bc..481d152 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
cpumask_copy(old, current-cpus_allowed);
for_each_online_cpu(cpu) {
- if (cpu == dying)
- continue;
+ WARN_ON_ONCE(cpu == dying);
Ok, I don't understand that:
we want to warn that the rediscovering is happening on a dying cpu?? And
before that, we simply jumped over it and didn't do the rediscovering
there? Why should we warn at all?
Huh?
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
which means the corresponding cpu has already dead. As a result, it
won't be accessed in the for_each_online_cpu loop.
So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
Signed-off-by: Tang Chen
---
arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 38e49bc..481d152 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
cpumask_copy(old, >cpus_allowed);
for_each_online_cpu(cpu) {
- if (cpu == dying)
- continue;
+ WARN_ON_ONCE(cpu == dying);
+
if (set_cpus_allowed_ptr(current, cpumask_of(cpu)))
continue;
/* Recheck banks in case CPUs don't all have the same */
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().
cmci_rediscover() is only called by the CPU_POST_DEAD event handler,
which means the corresponding cpu has already dead. As a result, it
won't be accessed in the for_each_online_cpu loop.
So, we could change the if(cpu == dying) statement into a WARN_ON_ONCE().
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
---
arch/x86/kernel/cpu/mcheck/mce_intel.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c
b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 38e49bc..481d152 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -180,8 +180,8 @@ void cmci_rediscover(int dying)
cpumask_copy(old, current-cpus_allowed);
for_each_online_cpu(cpu) {
- if (cpu == dying)
- continue;
+ WARN_ON_ONCE(cpu == dying);
+
if (set_cpus_allowed_ptr(current, cpumask_of(cpu)))
continue;
/* Recheck banks in case CPUs don't all have the same */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
