|From:||Linus Torvalds <torvalds-AT-linux-foundation.org>|
|To:||Rusty Russell <rusty-AT-rustcorp.com.au>|
|Subject:||Re: Fix quilt merge error in acpi-cpufreq.c|
|Date:||Wed, 15 Apr 2009 08:28:45 -0700 (PDT)|
|Cc:||Ingo Molnar <mingo-AT-elte.hu>, Linux Kernel Mailing List <linux-kernel-AT-vger.kernel.org>, Andrew Morton <akpm-AT-linux-foundation.org>, Dave Jones <davej-AT-redhat.com>|
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 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. (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. Linus
Copyright © 2009, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds