This goes on top of the cpu_active_map (take 2) patch.
Currently we depend on the stop_machine to provide nescessesary
synchronization for the cpu_active_map updates.
As Dmitry Adamushko pointed this is fragile and is not much clearer
than the previous scheme. In other words we do not want to depend on
the internal stop machine operation here.
So make the synchronization rules clear by doing synchronize_sched()
after clearing out cpu active bit.
Tested on quad-Core2 with:
while true; do
for i in 1 2 3; do
echo 0 > /sys/devices/system/cpu/cpu$i/online
done
for i in 1 2 3; do
echo 1 > /sys/devices/system/cpu/cpu$i/online
done
done
and
stress -c 200
No lockdep, preempt or other complaints.
Signed-off-by: Max Krasnyansky <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
kernel/cpu.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 71c5c9d..541f78e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -301,6 +301,14 @@ int __ref cpu_down(unsigned int cpu)
cpu_clear(cpu, cpu_active_map);
+ /* Make sure the all cpus did the reschedule and are not
+ * using stale version of the cpu_active_map.
+ * This is not strictly necessary becuase stop_machine()
+ * that we run down the line already provides the required
+ * synchronization. But it's really a side effect and we do not
+ * want to depend on the innards of the stop_machine here. */
+ synchronize_sched();
+
err = _cpu_down(cpu, 0);
if (cpu_online(cpu))
--
1.5.5.1
* Max Krasnyansky <[email protected]> wrote:
> This goes on top of the cpu_active_map (take 2) patch.
i've applied it to sched/devel. Note that checkpatch caught:
> + /* Make sure the all cpus did the reschedule and are not
> + * using stale version of the cpu_active_map.
> + * This is not strictly necessary becuase stop_machine()
> + * that we run down the line already provides the required
> + * synchronization. But it's really a side effect and we do not
> + * want to depend on the innards of the stop_machine here. */
> + synchronize_sched();
this had both whitespace problems (trailing spaces) and basic formatting
problems. I fixed these up.
Ingo
* Ingo Molnar <[email protected]> wrote:
> * Max Krasnyansky <[email protected]> wrote:
>
> > This goes on top of the cpu_active_map (take 2) patch.
>
> i've applied it to sched/devel. [...]
build fix below for UP.
Ingo
---------------------------------------------->
commit 1b427c153a08fdbc092c2bdbf845b92fda58d857
Author: Ingo Molnar <[email protected]>
Date: Fri Jul 18 14:01:39 2008 +0200
sched: fix build error, provide partition_sched_domains() unconditionally
provide an empty partition_sched_domains() definition for the UP case:
include/linux/cpuset.h: In function ‘rebuild_sched_domains':
include/linux/cpuset.h:163: error: implicit declaration of function ‘partition_sched_domains'
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/sched.h | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 1941d8b..26da921 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -824,7 +824,16 @@ extern void partition_sched_domains(int ndoms_new, cpumask_t *doms_new,
struct sched_domain_attr *dattr_new);
extern int arch_reinit_sched_domains(void);
-#endif /* CONFIG_SMP */
+#else /* CONFIG_SMP */
+
+struct sched_domain_attr;
+
+static inline void
+partition_sched_domains(int ndoms_new, cpumask_t *doms_new,
+ struct sched_domain_attr *dattr_new)
+{
+}
+#endif /* !CONFIG_SMP */
struct io_context; /* See blkdev.h */
#define NGROUPS_SMALL 32
Ingo Molnar wrote:
> * Ingo Molnar <[email protected]> wrote:
>
>> * Max Krasnyansky <[email protected]> wrote:
>>
>>> This goes on top of the cpu_active_map (take 2) patch.
>> i've applied it to sched/devel. [...]
>
> build fix below for UP.
Thanks for fixing this (and the spaces and stuff).
Max