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    [W:0.023 / U:1.232 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site