lkml.org 
[lkml]   [2012]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] x86, mce: Add persistent MCE event
On 03/22/2012 05:10 PM, Borislav Petkov wrote:

> On Thu, Mar 22, 2012 at 02:06:29PM +0530, Srivatsa S. Bhat wrote:
>>> +err_unwind:
>>> + err = -EINVAL;
>>> + for (--cpu; cpu >= 0; cpu--)
>>> + perf_rm_persistent_on_cpu(cpu, &per_cpu(mce_ev, cpu));
>>> +
>>
>>
>> *Totally* theoretical question: How do you know that the cpu_online_mask isn't
>> sparse? In other words, what if some CPUs weren't booted? Then this for-loop
>> wouldn't be very good..
>>
>> Oh, now I see that perf_rm_persistent_on_cpu() probably handles that case well..
>> So no issues I guess.. ?
>
> Right, this could theoretically come around to bite us in some obscure
> cases, so we probably fix it from the get-go.
>
>> (Moreover, we will probably have bigger issues at hand if some CPU didn't
>> boot..)
>>
>> (The code looked funny, so I thought of pointing it out, whether or not it
>> actually is worrisome. Sorry for the noise, if any).
>
> Right, no, thanks for pointing it out.
>
> I'll probably do something like the following:
>
> for (--cpu; cpu >= 0; cpu--)
> if (cpu_online(cpu))
> perf_rm_persistent_on_cpu(cpu, &per_cpu(mce_ev, cpu));
>
> to be on the safe side from that perspective.
>


You can do that or something like the following, to make it more readable:

int cpunum;

for_each_online_cpu(cpunum) {
if (cpunum == cpu)
break;
perf_rm_persistent_on_cpu(cpunum, &per_cpu(mce_ev, cpunum));
}
It is of course, up to you.. whichever form you prefer..

Regards,
Srivatsa S. Bhat



\
 
 \ /
  Last update: 2012-03-22 13:01    [from the cache]
©2003-2011 Jasper Spaans