* [PATCH] set_cpus_allowed() atomicity fix
@ 2002-10-09 23:05 Robert Love
2002-10-09 23:17 ` Linus Torvalds
0 siblings, 1 reply; 3+ messages in thread
From: Robert Love @ 2002-10-09 23:05 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel
L-Team Captain,
In set_cpus_allowed(), we hold the preempt_disable() over a
wait_for_completition() so it triggers the atomicity debugging.
There is (was) a kernel preemption race here which the preemption
disable is fixing. I do not understand it; if this uncovers it there
should be some light shed on the subject.
Anyhow, attached patch fixes the atomicity debugging error.
Patch is against current BK. Please, apply.
Robert Love
sched.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff -urN linux-bk/kernel/sched.c linux/kernel/sched.c
--- linux-bk/kernel/sched.c 2002-10-09 15:46:43.000000000 -0400
+++ linux/kernel/sched.c 2002-10-09 18:52:06.000000000 -0400
@@ -1953,7 +1953,6 @@
BUG();
#endif
- preempt_disable();
rq = task_rq_lock(p, &flags);
p->cpus_allowed = new_mask;
/*
@@ -1962,7 +1961,7 @@
*/
if (new_mask & (1UL << task_cpu(p))) {
task_rq_unlock(rq, &flags);
- goto out;
+ return;
}
/*
* If the task is not on a runqueue (and not running), then
@@ -1971,17 +1970,21 @@
if (!p->array && !task_running(rq, p)) {
set_task_cpu(p, __ffs(p->cpus_allowed));
task_rq_unlock(rq, &flags);
- goto out;
+ return;
}
init_completion(&req.done);
req.task = p;
list_add(&req.list, &rq->migration_queue);
+
+ /*
+ * counter the subsequent unlock - we do not want to preempt yet
+ */
+ preempt_disable();
task_rq_unlock(rq, &flags);
wake_up_process(rq->migration_thread);
+ preempt_enable();
wait_for_completion(&req.done);
-out:
- preempt_enable();
}
/*
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] set_cpus_allowed() atomicity fix
2002-10-09 23:05 [PATCH] set_cpus_allowed() atomicity fix Robert Love
@ 2002-10-09 23:17 ` Linus Torvalds
2002-10-10 1:05 ` Robert Love
0 siblings, 1 reply; 3+ messages in thread
From: Linus Torvalds @ 2002-10-09 23:17 UTC (permalink / raw)
To: Robert Love; +Cc: linux-kernel
On 9 Oct 2002, Robert Love wrote:
>
> Anyhow, attached patch fixes the atomicity debugging error.
I don't think this is right. You have to be preempt safe over the whole
time you're holding the "rq" pointer, I think. Otherwise you might move to
another CPU, at which point the rq state isn't valid any more. Or maybe I
misunderstood.
Linus
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] set_cpus_allowed() atomicity fix
2002-10-09 23:17 ` Linus Torvalds
@ 2002-10-10 1:05 ` Robert Love
0 siblings, 0 replies; 3+ messages in thread
From: Robert Love @ 2002-10-10 1:05 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel
On Wed, 2002-10-09 at 19:17, Linus Torvalds wrote:
> On 9 Oct 2002, Robert Love wrote:
> >
> > Anyhow, attached patch fixes the atomicity debugging error.
>
> I don't think this is right. You have to be preempt safe over the whole
> time you're holding the "rq" pointer, I think. Otherwise you might move to
> another CPU, at which point the rq state isn't valid any more. Or maybe I
> misunderstood.
I agree. But aren't we?
We are preempt-safe through the entire function (starting at the top
with task_rq_lock()) until the preempt_disable(). The only instruction
outside of the critical section is the wait_for_completion() which
sleeps anyhow.
Or maybe _I_ misunderstood?
Robert Love
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2002-10-10 0:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-09 23:05 [PATCH] set_cpus_allowed() atomicity fix Robert Love
2002-10-09 23:17 ` Linus Torvalds
2002-10-10 1:05 ` Robert Love
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).