mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Dmitry Adamushko <dmitry.adamushko@gmail.com>
Cc: Vegard Nossum <vegard.nossum@gmail.com>,
	Paul Menage <menage@google.com>,
	Max Krasnyansky <maxk@qualcomm.com>, Paul Jackson <pj@sgi.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	miaox@cn.fujitsu.com, 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: Sun, 13 Jul 2008 10:46:59 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807131041240.2959@woody.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.1.10.0807130943530.2959@woody.linux-foundation.org>



On Sun, 13 Jul 2008, Linus Torvalds wrote:
> 
> The thing is, we should fix the top level code to never even _consider_ an 
> invalid CPU as a target, and that in turn should mean that all the other 
> code should be able to just totally ignore CPU hotplug events.

IOW, I think we should totally remove the whole "update_sched_domains()" 
thing too. Any logic that needs it is broken. We shouldn't detach the 
scheduler domains in DOWN_PREPARE (much less UP_PREPARE), we should just 
leave them damn well alone.

As the comment says, "The domains and groups cannot be updated in place 
without racing with the balancing code". The thing is, we shouldn't even 
try. The correct way to handle all this is to make the balancing code use 
the domains regardless, but protect against CPU's going down with 
_another_ data structure that is much easier to update.

Namely something like 'cpu_active_map'.

Then we just get rid of all the crap in update_sched_domains() entirely, 
and then we can make the cpusets code do the *sane* thing, which is to 
rebuild the scheduler domains only when the CPU up/down has completed.

So instead of this illogical and crazy mess:

	+       switch (phase) {
	+       case CPU_UP_CANCELED:
	+       case CPU_UP_CANCELED_FROZEN:
	+       case CPU_DOWN_FAILED:
	+       case CPU_DOWN_FAILED_FROZEN:
	+       case CPU_ONLINE:
	+       case CPU_ONLINE_FROZEN:
	+       case CPU_DEAD:
	+       case CPU_DEAD_FROZEN:
	+               common_cpu_mem_hotplug_unplug(1);

it should just say

	+       switch (phase) {
	+       case CPU_ONLINE:
	+       case CPU_ONLINE_FROZEN:
	+       case CPU_DEAD:
	+       case CPU_DEAD_FROZEN:
	+               common_cpu_mem_hotplug_unplug(1);

because it only makes sense to rebuild the scheduler domains when the 
thing SUCCEEDS. 

See? By having a sane design, the code is not just more robust and easy to 
follow, you can also simplify it and make it more logical.

The current design is not sane.

			Linus

  parent reply	other threads:[~2008-07-13 17:47 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
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 [this message]
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.0807131041240.2959@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®