[lkml]   [2009]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Fix quilt merge error in acpi-cpufreq.c

    On Wed, 15 Apr 2009, Rusty Russell wrote:
    > The API is screwy. It excludes the current CPU from the mask,
    > unconditionally. It's a tlb flush helper masquerading as a general function.
    > (smp_call_function has the same issue).
    > Something like this?
    > Subject: smp_call_function_many: add explicit exclude_self flag

    No. This just makes the API even screwier. It fixes the
    "smp_processor_id()" thing, but it is

    (a) horribly buggy

    See the 'return' without any put_cpu()

    (b) horribly buggy


    if (exclude_self && cpu == this_cpu)
    cpu = cpumask_next_and(cpu, mask, cpu_online_mask);

    things are wrong - we need to do that "jump over our own CPU" thing
    regardless of whether 'exclude_self' is set or not, since we're going
    to special-case our own CPU anyway.

    (c) Wrong, even if it wasn't (horribly buggy)^2

    Adding "flags" to an interface doesn't make it better. Quite the
    reverse. It makes it worse. It also makes it even MORE different from
    all the other smp_call_function's, which just do the 'self' cpu
    without any stupid conditionals and flags.

    IOW, it would make things worse even if it worked. Which it doesn't.

    > Impact: clarify and extend confusing API

    And what the hell is up with these bogus "Impact:" things? Who started
    doing that, and why? If your single-line explanation at the top is not
    good enough, and your multi-line explanation isn't clear enough, then you
    should fix the OTHER parts, not add that _idiotic_ "Impact" statement.

    The thing has spread like wildfire, and it's STUPID.


     \ /
      Last update: 2009-04-15 17:35    [W:0.021 / U:113.328 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site