mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Ingo Molnar <mingo@elte.hu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Jones <davej@redhat.com>
Subject: Re: Fix quilt merge error in acpi-cpufreq.c
Date: Wed, 15 Apr 2009 08:28:45 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.00.0904150822020.4132@localhost.localdomain> (raw)
In-Reply-To: <200904152014.11717.rusty@rustcorp.com.au>



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

  reply	other threads:[~2009-04-15 15:31 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200904140159.n3E1x1K1014705@hera.kernel.org>
2009-04-14  2:05 ` Ingo Molnar
2009-04-15  5:44   ` Ingo Molnar
2009-04-15 10:44     ` Rusty Russell
2009-04-15 15:28       ` Linus Torvalds [this message]
2009-04-15 16:26         ` Ingo Molnar
2009-04-15 16:46           ` H. Peter Anvin
2009-04-15 17:00             ` H. Peter Anvin
2009-04-15 17:19           ` Linus Torvalds
2009-04-15 18:47             ` H. Peter Anvin
2009-04-15 19:43               ` Linus Torvalds
2009-04-15 20:07                 ` Ingo Molnar
2009-04-15 20:32                 ` Andrew Morton
2009-04-15 21:03                   ` Ingo Molnar
2009-04-15 21:15                     ` Linus Torvalds
2009-04-15 22:40                       ` Ingo Molnar
2009-04-15 23:08                         ` Linus Torvalds
2009-04-16  0:08                           ` Ingo Molnar
2009-04-16  0:23                             ` Linus Torvalds
2009-04-16  0:38                               ` Linus Torvalds
2009-04-16  0:50                                 ` Ingo Molnar
2009-04-16  4:33                                   ` H. Peter Anvin
2009-04-16  7:14                                     ` Ingo Molnar
2009-04-16 15:24                                   ` Valdis.Kletnieks
2009-04-15 23:49                       ` David Miller
2009-04-16 11:00                         ` Christoph Hellwig
2009-04-15 21:17                     ` Andrew Morton
2009-04-15 23:04                       ` Ingo Molnar
2009-04-15 21:23                 ` David Miller
2009-04-15 22:48                   ` Ingo Molnar
2009-04-15 23:11                     ` Linus Torvalds
2009-04-16  0:44                       ` Ingo Molnar
2009-04-16  1:03                         ` Linus Torvalds
2009-04-16  1:46                           ` Ingo Molnar
2009-04-16  2:22                             ` Linus Torvalds
2009-04-16  7:23                               ` Ingo Molnar
2009-04-16  3:55                             ` Theodore Tso
2009-04-16  7:44                               ` Ingo Molnar
2009-04-16 15:41                             ` Valdis.Kletnieks
2009-04-16 13:04                   ` Valdis.Kletnieks
2009-04-16  2:00               ` Rusty Russell
2009-04-16  2:22                 ` Paul Gortmaker
2009-04-16  2:34                 ` Linus Torvalds
2009-04-16  3:10                 ` Ray Lee
2009-04-16  7:56                 ` Ingo Molnar
2009-04-16 11:57                   ` Theodore Tso
2009-04-16 13:55                 ` Jonathan Corbet
2009-04-20  8:14                   ` Rusty Russell
2009-04-20 10:38                     ` Ingo Molnar
2009-04-22  4:18                       ` Rusty Russell
2009-04-21 19:37                     ` Jonathan Corbet
2009-04-22  1:58                       ` Rusty Russell
2009-04-16  1:27         ` Rusty Russell
2009-04-16  2:31           ` Theodore Tso
2009-04-16  8:02             ` Ingo Molnar
2009-04-15 15:05     ` Linus Torvalds
2009-04-15 15:22       ` Ali Gholami Rudi
2009-04-15 16:41       ` Ingo Molnar
     [not found] <crh66-6nQ-7@gated-at.bofh.it>
     [not found] ` <crilu-8hM-13@gated-at.bofh.it>
     [not found]   ` <crjhu-1lb-13@gated-at.bofh.it>
     [not found]     ` <crkQl-3QL-7@gated-at.bofh.it>
     [not found]       ` <crm5K-5NR-17@gated-at.bofh.it>
     [not found]         ` <crmyK-6DP-9@gated-at.bofh.it>
     [not found]           ` <crnXV-g5-27@gated-at.bofh.it>
     [not found]             ` <croh9-VK-5@gated-at.bofh.it>
     [not found]               ` <croTQ-1Jm-1@gated-at.bofh.it>
     [not found]                 ` <crqVM-4UC-11@gated-at.bofh.it>
2009-04-16  5:46                   ` Niel Lambrechts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.00.0904150822020.4132@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome