mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Miao Xie <miaox@cn.fujitsu.com>
Cc: Vegard Nossum <vegard.nossum@gmail.com>,
	Dmitry Adamushko <dmitry.adamushko@gmail.com>,
	Paul Menage <menage@google.com>,
	Max Krasnyansky <maxk@qualcomm.com>, Paul Jackson <pj@sgi.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	rostedt@goodmis.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@elte.hu>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: current linux-2.6.git: cpusets completely broken
Date: Sat, 12 Jul 2008 12:15:06 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807121201220.2875@woody.linux-foundation.org> (raw)
In-Reply-To: <487880D7.1040608@cn.fujitsu.com>



On Sat, 12 Jul 2008, Miao Xie wrote:
> 
> I think Vegard Nossum's patch is not so good because it is not necessary to detach
> all the sched domains when making a cpu offline.

Well, short-term, I'd like to have a minimal fix.

Long-term, I really think that whole CPU notifier thing needs to be 
*fixed*.

It's totally broken to have single callback functions with "flags" 
parameters telling people what to do. I don't understand why people keep 
doing it. It's *always* broken, and the fix is *always* to a structure 
with multiple function pointers - separate functions for separate events.

In the case of those CPU hotplug notifiers, the "freeze" thing could 
probably have been a flag, but CPU_DOWN_PREPARE/DEAD/DYING/UP/xyz should 
likely just be different function callbacks. That way, when we add a 
callback type, it doesn't require changing all existing callbacks that 
don't want to care about the new case. And we wouldn't have these stupid 
and fragile and ugly "switch()" statements with tons of cases all over.

Sadly, people have latched onto a model (that piece-of-sh*t "notifier" 
infrastructure) that encourages - and almost requires - this kind of pure 
crap.

But the CPU hotplug stuff _could_ just use separate notifier chains for 
the different kinds of events. It wouldn't be perfect, but it would be 
better than the mess we have now. Instead of doing

	register_cpu_notifier(..);

and having a single thing that has to handle all cases, we could have

	/* current "ONLINE/ONLINE_FROZEN" cases */
	online = register_cpu_notifier(CPU_ONLINE, online_fn);
	dead = register_cpu_nofitier(CPU_DEAD, dead_fn);

which would allocate the "struct notifier_block" an fill it in, and have 
_separate_ queues for all those cases. That way, *if* you want to share 
the code for multiple cases, you just register the same function. And if 
you only cared about one case, you'd only be called for that one case!

I dunno. Maybe the conversion would be painful. And maybe the end result 
isn't wonderful either. But the current setup for CPU notifiers is just a 
damn disgrace.

			Linus

  parent reply	other threads:[~2008-07-12 19:15 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-11 19:07 Vegard Nossum
2008-07-11 19:36 ` Paul Menage
2008-07-11 19:43   ` Vegard Nossum
2008-07-11 20:07     ` Max Krasnyansky
2008-07-11 23:03     ` Dmitry Adamushko
2008-07-11 23:19       ` Max Krasnyansky
2008-07-11 23:53         ` Dmitry Adamushko
2008-07-12  3:17       ` Vegard Nossum
2008-07-12  3:28         ` Linus Torvalds
2008-07-12 10:00           ` Miao Xie
2008-07-12 11:05             ` Dmitry Adamushko
2008-07-12 19:15             ` Linus Torvalds [this message]
2008-07-12 10:04           ` Dmitry Adamushko
2008-07-12 19:19             ` Max Krasnyansky
2008-07-12 20:10             ` Linus Torvalds
2008-07-12 21:30               ` Linus Torvalds
2008-07-12 22:07                 ` Linus Torvalds
2008-07-12 22:43                   ` Max Krasnyansky
2008-07-12 23:01                     ` Linus Torvalds
2008-07-12 23:00                   ` Vegard Nossum
2008-07-12 23:04                     ` Linus Torvalds
2008-07-12 23:19                       ` Dmitry Adamushko
2008-07-12 23:25                         ` Dmitry Adamushko
2008-07-12 23:05                     ` Dmitry Adamushko
2008-07-12 23:17                       ` Linus Torvalds
2008-07-13  9:53                         ` Dmitry Adamushko
2008-07-13 17:10                           ` Linus Torvalds
2008-07-13 17:42                             ` Ingo Molnar
2008-07-13 17:46                             ` Linus Torvalds
2008-07-13 18:13                               ` Dmitry Adamushko
2008-07-13 18:19                                 ` Ingo Molnar
2008-07-13 18:38                                   ` Linus Torvalds
2008-07-13 18:20                                 ` Linus Torvalds
2008-07-12 23:25                       ` Vegard Nossum
2008-07-13 15:29                 ` Andi Kleen
2008-07-14 15:49                   ` Mike Travis
2008-07-14 22:38                 ` Dmitry Adamushko
2008-07-14 23:05                   ` Linus Torvalds
2008-07-15  0:00                     ` Dmitry Adamushko
2008-07-15  0:23                       ` Linus Torvalds
2008-07-15  2:21                         ` Dmitry Adamushko
2008-07-15  3:03                           ` Max Krasnyansky
2008-07-15  4:12                             ` Linus Torvalds
2008-07-15  8:32                               ` Ingo Molnar
2008-07-15  8:42                                 ` Max Krasnyansky
2008-07-15  8:57                                   ` Ingo Molnar
2008-07-15  9:12                                     ` Max Krasnyansky
2008-07-16  6:35                                     ` Max Krasnyansky
2008-07-16  7:10                                       ` Peter Zijlstra
2008-07-16 17:01                                         ` Max Krasnyansky
2008-07-15  3:23                     ` Steven Rostedt
2008-07-15  3:36                       ` Linus Torvalds
2008-07-15  3:47                         ` Steven Rostedt
2008-07-15  4:04                           ` Linus Torvalds
2008-07-15  4:16                             ` Steven Rostedt
2008-07-12 10:45 Dmitry Adamushko
2008-07-12 11:14 ` Dmitry Adamushko
2008-07-13  0:10   ` Dmitry Adamushko
2008-07-13  8:50     ` Vegard Nossum
2008-07-13  9:41       ` Ingo Molnar

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.1.10.0807121201220.2875@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=dmitry.adamushko@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxk@qualcomm.com \
    --cc=menage@google.com \
    --cc=miaox@cn.fujitsu.com \
    --cc=mingo@elte.hu \
    --cc=pj@sgi.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=vegard.nossum@gmail.com \
    /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

all inboxes | Powered by JetHome®