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: Mon, 14 Jul 2008 17:23:38 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807141707410.3017@woody.linux-foundation.org> (raw)
In-Reply-To: <b647ffbd0807141700s20e54fbewafb3d3e296e57f53@mail.gmail.com>



On Tue, 15 Jul 2008, Dmitry Adamushko wrote:
> 
> The 'synchronization' point occurs even earlier - when cpu_down() ->
> __stop_machine_run() gets called (as I described in my previous mail).
> 
> My point was that if it's ok to have a _delayed_ synchronization
> point, having it not immediately after cpu_clear(cpu, cpu_active_map)
> but when the "runqueue lock" is taken a bit later (as you pointed out
> above) or __stop_machine_run() gets executed (which is a sync point,
> scheduling-wise),
> 
> then we can implement the proper synchronization (hotplugging vs.
> task-migration) with cpu_online_map (no need for cpu_active_map).

Maybe. But what is the point? And more importantly, I think it's wrong.

There's really a *difference* between "this CPU is still running, but 
going down" and "this CPU is running".

And it's a valid difference. For example, a process should be able to 
absolutely DEPEND on being able to depend on cpu_online(current_cpu()) 
*always* being true. 

I also don't understand why people are arguing against a single new CPU 
map (it's _global_ to the whole kernel, for crissake!) when it clearly 
makes the rules much simpler. Look at the patch I sent out, and tell me it 
isn't 100% obvious what cpu_active_map does, and what the logic is.

In contrast, try to follow the same for cpu_online_map. I dare you. You 
have to already know that code really really well in order to understand 
what happens to it, both at bootup _and_ at hotplug events.

Dmitry, THIS CODE WAS BUGGY. Not just once. Multiple f*cking times!

That should tell you something.

In particular, it should tell you that the code is too hard to follow, and 
too fragile, and a total mess.

I do NOT understand why you seem to argue for being "subtle" and "clever", 
considering the history of this whole setup. Subtle and clever and complex 
is what got us to the crap situation.

So here's the code-word of today: KISS - Keep It Simple Stupid.

And I _guarantee_ that the "cpu_active_map" approach is a hell of a lot 
simpler than the alternatives. Partly because it really matches what we 
want much more closely: it gives a clear new state for "this CPU is going 
down, even though things are still running on it".

And then it's 100% logical to say: "ok, if it's going down, we agree to 
not add new processes to it".

THAT is the kind of logic we should strive for. Not "let's avoid the 
obvious and simple code because we can re-use the existing messy code for 
yet another thing".

Dammit, this code should be easier to understand, not harder!

		Linus

  reply	other threads:[~2008-07-15  0:24 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
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 [this message]
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.0807141707410.3017@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®