linux-kernel - Re: Fix quilt merge error in acpi-cpufreq.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0904150822020.4132@localhost.localdomain>
Date:	Wed, 15 Apr 2009 08:28:45 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Rusty Russell <rusty@...tcorp.com.au>
cc:	Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Jones <davej@...hat.com>
Subject: Re: 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

     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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/