Re: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Srivatsa S. Bhat
On 09/26/2012 01:40 PM, Liu, Chuansheng wrote: >> Btw, on a slightly different note, I'm also rather surprised that the above >> code doesn't care about the return value of chip->irq_set_affinity() .. >> Shouldn't we warn if that fails? > > It seems another case when irq_set_affinity is NULL whene

RE: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Liu, Chuansheng
> I know.. What I meant is, the code warns only if chip->irq_set_affinity > is NULL and doesn't care if chip->irq_set_affinity was not NULL and > the function failed to set the affinity (ie., when chip->irq_set_affinity() > returns error). In other words, I meant to say that this is one more > case

Re: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Srivatsa S. Bhat
On 09/26/2012 01:47 PM, Liu, Chuansheng wrote: >> Shouldn't we warn if that fails? > printk("Cannot set affinity for irq %i\n", irq); > This is the warning when set affinity failed. > I know.. What I meant is, the code warns only if chip->irq_set_affinity is NULL and doesn't care if chip->irq_set

RE: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Liu, Chuansheng
> In that case, we would end up with an incorrect data->affinity right? > Moving the clean cpu mask code into if (chip->irq_set_affinity)? Will resend the patch and will judge the chip->irq_set_affinity(data, affinity, true) return value.

RE: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Liu, Chuansheng
> Shouldn't we warn if that fails? printk("Cannot set affinity for irq %i\n", irq); This is the warning when set affinity failed.

RE: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Liu, Chuansheng
> Btw, on a slightly different note, I'm also rather surprised that the above > code doesn't care about the return value of chip->irq_set_affinity() .. > Shouldn't we warn if that fails? It seems another case when irq_set_affinity is NULL whenever affinity is changed or not before that, For this

Re: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-26 Thread Srivatsa S. Bhat
On 09/26/2012 12:22 PM, Liu, Chuansheng wrote: >>> + } else if (cpumask_test_cpu(cpu, data->affinity)) >>> + cpumask_clear_cpu(cpu, data->affinity); >>> >> >> You meant to use 'affinity' (instead of data->affinity) in the above 2 >> statements >> right? Note that we do

RE: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-25 Thread Liu, Chuansheng
> > + } else if (cpumask_test_cpu(cpu, data->affinity)) > > + cpumask_clear_cpu(cpu, data->affinity); > > > > You meant to use 'affinity' (instead of data->affinity) in the above 2 > statements > right? Note that we do chip->irq_set_affinity(data, affinity, true); furt

Re: [PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-25 Thread Srivatsa S. Bhat
On 09/26/2012 08:02 PM, Chuansheng Liu wrote: > > When one CPU is going offline, and fixup_irqs() will re-set the > irq affinity in some cases, we should clean the offlining CPU from > the irq affinity. > > The reason is setting offlining CPU as of the affinity is useless. > Moreover, the smp_aff

[PATCH] x86/fixup_irq: Clean the offlining CPU from the irq affinity mask

2012-09-25 Thread Chuansheng Liu
When one CPU is going offline, and fixup_irqs() will re-set the irq affinity in some cases, we should clean the offlining CPU from the irq affinity. The reason is setting offlining CPU as of the affinity is useless. Moreover, the smp_affinity value will be confusing when the offlining CPU come ba