lkml.org 
[lkml]   [2009]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: Fix quilt merge error in acpi-cpufreq.c
    Date
    On Thu, 16 Apr 2009 12:58:45 am Linus Torvalds wrote:
    >
    > 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

    Sure. Did it even compile?

    > Those
    >
    > 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.

    I don't think so (smp_call_function_single will DTRT if
    cpu == smp_processor_id). But I didn't test to be sure.

    > (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.

    Uglier. Worse? It would have prevented Andrew's mistake.

    > 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.

    You've said this twice, but unfortunately that doesn't make it true.

    smp_call_function() is the original from which this derives, and it has
    always skipped the current cpu. Hence on_each_cpu().

    I'd love to see a fix which isn't ugly and doesn't put a cpumask on the
    stack.

    > > Impact: clarify and extend confusing API
    >
    > And what the hell is up with these bogus "Impact:" things? Who started
    > doing that, and why?

    Ingo wants them. Example:

    lguest: don't expect linear addresses in gdt pvops

    Impact: fix guest crash 'lguest: bad read address 0x4800000 len 256'

    What's more important in the subject line? That it fixes a crash, or what it
    does?

    Thanks,
    Rusty.


    \
     
     \ /
      Last update: 2009-04-16 03:31    [W:0.031 / U:0.228 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site