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 for Android: free password hash cracker in your pocket

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

Message-Id: <200904161057.07108.rusty@rustcorp.com.au>
Date:	Thu, 16 Apr 2009 10:57:05 +0930
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
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 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.
--
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/