Messages in this thread | | | Date | Thu, 22 Mar 2012 17:27:48 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [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
| |