Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Tang Chen
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

RE: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Luck, Tony
> 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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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. --

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Miao Xie
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.

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Miao Xie
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()

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Miao Xie
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Miao Xie
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.

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Borislav Petkov
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,

RE: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Luck, Tony
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-23 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Borislav Petkov
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,

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Borislav Petkov
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-22 Thread Tang Chen
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

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-21 Thread Tang Chen
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.

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-21 Thread Tang Chen
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.

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-19 Thread Borislav Petkov
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)

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-19 Thread Greg KH
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)

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-19 Thread Greg KH
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)

Re: [PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-19 Thread Borislav Petkov
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)

[PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-18 Thread Tang Chen
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 ---

[PATCH v2 1/2] Replace if statement with WARN_ON_ONCE() in cmci_rediscover().

2012-10-18 Thread Tang Chen
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