* [PATCH] use for_each_cpu
@ 2004-08-01 6:01 Anton Blanchard
2004-08-01 6:08 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Anton Blanchard @ 2004-08-01 6:01 UTC (permalink / raw)
To: akpm, rusty; +Cc: linux-kernel
Hi,
The per cpu schedule counters need to be summed up over all possible cpus.
When testing hotplug cpu remove I saw the sum of the online cpu count
for nr_uninterruptible go negative which made the load average go nuts.
Anton
diff -puN kernel/sched.c~debug_nr_running kernel/sched.c
--- foobar2/kernel/sched.c~debug_nr_running 2004-08-01 14:42:46.133968016 +1000
+++ foobar2-anton/kernel/sched.c 2004-08-01 15:26:17.272626720 +1000
@@ -1095,7 +1095,7 @@ unsigned long nr_uninterruptible(void)
{
unsigned long i, sum = 0;
- for_each_online_cpu(i)
+ for_each_cpu(i)
sum += cpu_rq(i)->nr_uninterruptible;
return sum;
@@ -1105,7 +1105,7 @@ unsigned long long nr_context_switches(v
{
unsigned long long i, sum = 0;
- for_each_online_cpu(i)
+ for_each_cpu(i)
sum += cpu_rq(i)->nr_switches;
return sum;
@@ -1115,7 +1115,7 @@ unsigned long nr_iowait(void)
{
unsigned long i, sum = 0;
- for_each_online_cpu(i)
+ for_each_cpu(i)
sum += atomic_read(&cpu_rq(i)->nr_iowait);
return sum;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] use for_each_cpu
2004-08-01 6:01 [PATCH] use for_each_cpu Anton Blanchard
@ 2004-08-01 6:08 ` Andrew Morton
2004-08-01 7:27 ` Anton Blanchard
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2004-08-01 6:08 UTC (permalink / raw)
To: Anton Blanchard; +Cc: rusty, linux-kernel
Anton Blanchard <anton@samba.org> wrote:
>
> The per cpu schedule counters need to be summed up over all possible cpus.
> When testing hotplug cpu remove I saw the sum of the online cpu count
> for nr_uninterruptible go negative which made the load average go nuts.
I think the preferred approach here is to transfer the count over to the
current CPU in the CPU_DEAD handler.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] use for_each_cpu
2004-08-01 6:08 ` Andrew Morton
@ 2004-08-01 7:27 ` Anton Blanchard
2004-08-01 7:47 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Anton Blanchard @ 2004-08-01 7:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: rusty, linux-kernel
> > The per cpu schedule counters need to be summed up over all possible cpus.
> > When testing hotplug cpu remove I saw the sum of the online cpu count
> > for nr_uninterruptible go negative which made the load average go nuts.
>
> I think the preferred approach here is to transfer the count over to the
> current CPU in the CPU_DEAD handler.
They only look to be called out of proc, and once every 5 seconds for
loadaverage calculations. Is it worth adding complexity to the cpu
notifiers vs just using for_each_cpu?
Anton
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] use for_each_cpu
2004-08-01 7:27 ` Anton Blanchard
@ 2004-08-01 7:47 ` Andrew Morton
2004-08-01 21:00 ` Anton Blanchard
2004-08-02 5:59 ` Rusty Russell
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2004-08-01 7:47 UTC (permalink / raw)
To: Anton Blanchard; +Cc: rusty, linux-kernel
Anton Blanchard <anton@samba.org> wrote:
>
>
> > > The per cpu schedule counters need to be summed up over all possible cpus.
> > > When testing hotplug cpu remove I saw the sum of the online cpu count
> > > for nr_uninterruptible go negative which made the load average go nuts.
> >
> > I think the preferred approach here is to transfer the count over to the
> > current CPU in the CPU_DEAD handler.
>
> They only look to be called out of proc, and once every 5 seconds for
> loadaverage calculations.
OK.
> Is it worth adding complexity to the cpu
> notifiers vs just using for_each_cpu?
yup ;) It's only six lines, and it follows the same pattern as is used in,
say, page_alloc_cpu_notify(). Doing the same thing the same way in
multiple places is to be preferred, yes?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] use for_each_cpu
2004-08-01 7:47 ` Andrew Morton
@ 2004-08-01 21:00 ` Anton Blanchard
2004-08-02 5:59 ` Rusty Russell
1 sibling, 0 replies; 6+ messages in thread
From: Anton Blanchard @ 2004-08-01 21:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: rusty, linux-kernel
> yup ;) It's only six lines, and it follows the same pattern as is used in,
> say, page_alloc_cpu_notify(). Doing the same thing the same way in
> multiple places is to be preferred, yes?
If the data structure contains allocated memory, like per cpu pages I
agree. But for percpu stuff that is straight stats (eg nr_running), I
thought the aim was to just total all all possible cpus. bh_accounting
is another one that falls into this group.
Anton
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] use for_each_cpu
2004-08-01 7:47 ` Andrew Morton
2004-08-01 21:00 ` Anton Blanchard
@ 2004-08-02 5:59 ` Rusty Russell
1 sibling, 0 replies; 6+ messages in thread
From: Rusty Russell @ 2004-08-02 5:59 UTC (permalink / raw)
To: Andrew Morton; +Cc: Anton Blanchard, lkml - Kernel Mailing List
On Sun, 2004-08-01 at 17:47, Andrew Morton wrote:
> > Is it worth adding complexity to the cpu
> > notifiers vs just using for_each_cpu?
>
> yup ;) It's only six lines, and it follows the same pattern as is used in,
> say, page_alloc_cpu_notify(). Doing the same thing the same way in
> multiple places is to be preferred, yes?
Yes, and that way is "for_each_cpu".
Andrew, take his patch. You know for_each_cpu is preferred over
for_each_online_cpu unless there's a good reason for the latter.
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-08-02 6:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-01 6:01 [PATCH] use for_each_cpu Anton Blanchard
2004-08-01 6:08 ` Andrew Morton
2004-08-01 7:27 ` Anton Blanchard
2004-08-01 7:47 ` Andrew Morton
2004-08-01 21:00 ` Anton Blanchard
2004-08-02 5:59 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox