2004-09-08 13:59:57

by Nick Piggin

[permalink] [raw]
Subject: [PATCH 1/3] sched: trivial changes

Couple of trival changes before the hotplug stuff.


Attachments:
sched-trivial.patch (2.02 kB)

2004-09-08 14:34:10

by Nick Piggin

[permalink] [raw]
Subject: [PATCH 3/3] sched: cpu hotplug notifier for updating sched domains

3/3

This builds on (and is mostly) Nathan's work. I have messed with a few
things though. It survives some cpu hotplugging on i386. Nathan, can you
give this a look? Do you agree with the general direction?

Thanks


Attachments:
sched-domains-cpu-hotplug-notifier.patch (13.57 kB)

2004-09-08 16:02:39

by Nathan Lynch

[permalink] [raw]
Subject: [PATCH] fix schedstats null deref in sched_exec

Oops, I meant to send this one with my original patchset.

In sched_exec, schedstat_inc will dereference a null pointer if no
domain is found with the SD_BALANCE_EXEC flag set. This was exposed
during testing of the previous patches where cpus are temporarily
attached to a dummy domain without SD_BALANCE_EXEC set.

Signed-off-by: Nathan Lynch <[email protected]>


---


diff -puN kernel/sched.c~sched_exec-schedstats-fix kernel/sched.c
--- 2.6.9-rc1-bk12/kernel/sched.c~sched_exec-schedstats-fix 2004-09-06 02:14:05.000000000 -0500
+++ 2.6.9-rc1-bk12-nathanl/kernel/sched.c 2004-09-06 19:00:12.000000000 -0500
@@ -1727,8 +1727,8 @@ void sched_exec(void)
if (tmp->flags & SD_BALANCE_EXEC)
sd = tmp;

- schedstat_inc(sd, sbe_attempts);
if (sd) {
+ schedstat_inc(sd, sbe_attempts);
new_cpu = find_idlest_cpu(current, this_cpu, sd);
if (new_cpu != this_cpu) {
schedstat_inc(sd, sbe_pushed);

_


2004-09-08 20:00:01

by Nathan Lynch

[permalink] [raw]
Subject: Re: [PATCH 3/3] sched: cpu hotplug notifier for updating sched domains

On Wed, 2004-09-08 at 07:57, Nick Piggin wrote:
> 3/3
>
> This builds on (and is mostly) Nathan's work. I have messed with a few
> things though. It survives some cpu hotplugging on i386. Nathan, can you
> give this a look? Do you agree with the general direction?

Yep, looks good to me, and I gave it a sniff test on ppc64. Thanks.

Nathan


2004-09-09 00:40:00

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH] fix schedstats null deref in sched_exec

Nathan Lynch wrote:
> Oops, I meant to send this one with my original patchset.
>
> In sched_exec, schedstat_inc will dereference a null pointer if no
> domain is found with the SD_BALANCE_EXEC flag set. This was exposed
> during testing of the previous patches where cpus are temporarily
> attached to a dummy domain without SD_BALANCE_EXEC set.
>
> Signed-off-by: Nathan Lynch <[email protected]>
>

Thanks. I'll get everything together and send it to Andrew.

2004-09-09 10:28:22

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH 2/3] cpu: add a CPU_DOWN_PREPARE notifier

On Wed, 2004-09-08 at 22:52, Nick Piggin wrote:
> 2/3
>
> Rusty, can I do this?
>
> ______________________________________________________________________
> Add a CPU_DOWN_PREPARE hotplug CPU notifier. This is needed so we can
> dettach all sched-domains before a CPU goes down, thus we can build
> domains from online cpumasks, and not have to check for the possibility
> of a CPU coming up or going down.

And if taking the CPU down fails? If you need this, you need the
CPU_DOWN_FAILED as well, unfortunately. Hence I prefer the "do the
domain thing while machine is frozen" and sidestep it entirely.

Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell

2004-09-09 10:41:36

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH 2/3] cpu: add a CPU_DOWN_PREPARE notifier

Rusty Russell wrote:
> On Wed, 2004-09-08 at 22:52, Nick Piggin wrote:
>
>>2/3
>>
>>Rusty, can I do this?
>>
>>______________________________________________________________________
>>Add a CPU_DOWN_PREPARE hotplug CPU notifier. This is needed so we can
>>dettach all sched-domains before a CPU goes down, thus we can build
>>domains from online cpumasks, and not have to check for the possibility
>>of a CPU coming up or going down.
>
>
> And if taking the CPU down fails? If you need this, you need the
> CPU_DOWN_FAILED as well, unfortunately. Hence I prefer the "do the
> domain thing while machine is frozen" and sidestep it entirely.
>

Really? It doesn't need to be run from the stop_machine_run
context at all - it can happily be done while the system is
running.

That said, if you really object to CPU_DOWN_PREPARE and CPU_DOWN_FAILED,
it probably shouldn't be too much work. Should it make the call from
take_cpu_down?

2004-09-09 11:16:37

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH 2/3] cpu: add a CPU_DOWN_PREPARE notifier

Nick Piggin wrote:
> Rusty Russell wrote:
>
>> On Wed, 2004-09-08 at 22:52, Nick Piggin wrote:
>>
>>> 2/3
>>>
>>> Rusty, can I do this?
>>>
>>> ______________________________________________________________________
>>> Add a CPU_DOWN_PREPARE hotplug CPU notifier. This is needed so we can
>>> dettach all sched-domains before a CPU goes down, thus we can build
>>> domains from online cpumasks, and not have to check for the possibility
>>> of a CPU coming up or going down.
>>
>>
>>
>> And if taking the CPU down fails? If you need this, you need the
>> CPU_DOWN_FAILED as well, unfortunately. Hence I prefer the "do the
>> domain thing while machine is frozen" and sidestep it entirely.
>>
>
> Really? It doesn't need to be run from the stop_machine_run
> context at all - it can happily be done while the system is
> running.
>
> That said, if you really object to CPU_DOWN_PREPARE and CPU_DOWN_FAILED,
> it probably shouldn't be too much work. Should it make the call from
> take_cpu_down?
>

The other thing is, it is actually a lot nicer to have this done while
the machine is running, because then cpu_attach_domain guarantees a
quiescent state, so the attach can be done completely atomically from
the point of view of the rest of the scheduler.

So you can always rely on domains and cpu_online_map staying in synch.
The alternative is more fastpath checking of the cpu_online_map, and
possibly more hotplug locks.

In short, I'd really like to have it done this way.