From: Gregory Haskins <ghaskins@novell.com>
To: linux-kernel@vger.kernel.org
Cc: peterz@infradead.org, rostedt@goodmis.org, rusty@rustcorp.com.au,
maxk@qualcomm.com, mingo@elte.hu
Subject: [PATCH 2/2] sched: fix race in cpupri introduced by cpumask_var changes
Date: Thu, 30 Jul 2009 10:57:28 -0400 [thread overview]
Message-ID: <20090730145728.25226.92769.stgit@dev.haskins.net> (raw)
In-Reply-To: <20090730145623.25226.29033.stgit@dev.haskins.net>
[
Note: obsoletes c8cab1ddfafd112c82a69a6cc4ec608984eeac97,
though it should be harmless to leave the old patch applied.
]
Background:
Several race conditions in the scheduler have cropped up recently,
which Steven and I have tracked down using ftrace. The most recent
one turns out to be a race in how the scheduler determines a
suitable migration target for RT tasks, introduced recently with commit:
commit 68e74568fbe5854952355e942acca51f138096d9
Author: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue Nov 25 02:35:13 2008 +1030
sched: convert struct cpupri_vec cpumask_var_t.
The original design of cpupri allowed lockless readers to quickly
determine a best-estimate target. Races between the pri_active
bitmap and the vec->mask were handled in the original code because
we would detect and return "0" when this occured. The design was
predicated on the *effective* atomicity (*) of caching the result
of cpus_and() between the cpus_allowed and the vec->mask.
Commit 68e74568 changed the behavior such that vec->mask is accessed
multiple times. This introduces a subtle race, the result of which
means we can have a result that returns "1", but with an empty bitmap.
*) yes, we know cpus_and() is not a locked operator across the entire
composite array, but it is implicitly atomic on a per-word basis
which is all the design required to work.
Implementation:
Rather than forgoing the lockless design, or reverting to a stack-based
cpumask_t, we simply check for when the race has been encountered and
continue processing in the event that the race is hit. This renders the
removal race as if the priority bit had been atomically cleared as well,
and allows the algorithm to execute correctly.
Signed-off-by: Gregory Haskins <ghaskins@novell.com>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Steven Rostedt <srostedt@redhat.com>
---
kernel/sched_cpupri.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/kernel/sched_cpupri.c b/kernel/sched_cpupri.c
index aef68b6..0f052fc 100644
--- a/kernel/sched_cpupri.c
+++ b/kernel/sched_cpupri.c
@@ -81,8 +81,21 @@ int cpupri_find(struct cpupri *cp, struct task_struct *p,
if (cpumask_any_and(&p->cpus_allowed, vec->mask) >= nr_cpu_ids)
continue;
- if (lowest_mask)
+ if (lowest_mask) {
cpumask_and(lowest_mask, &p->cpus_allowed, vec->mask);
+
+ /*
+ * We have to ensure that we have at least one bit
+ * still set in the array, since the map could have
+ * been concurrently emptied between the first and
+ * second reads of vec->mask. If we hit this
+ * condition, simply act as though we never hit this
+ * priority level and continue on.
+ */
+ if (cpumask_any(lowest_mask) >= nr_cpu_ids)
+ continue;
+ }
+
return 1;
}
next prev parent reply other threads:[~2009-07-30 14:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-30 14:57 [PATCH 0/2] scheduler fixes Gregory Haskins
2009-07-30 14:57 ` [PATCH 1/2] [RESEND] sched: Fully integrate cpus_active_map and root-domain code Gregory Haskins
2009-07-30 15:01 ` Gregory Haskins
2009-07-30 15:10 ` Gregory Haskins
2009-08-02 13:13 ` [tip:sched/core] " tip-bot for Gregory Haskins
2009-07-30 14:57 ` Gregory Haskins [this message]
2009-08-02 13:12 ` [tip:sched/core] sched: Fix race in cpupri introduced by cpumask_var changes tip-bot for Gregory Haskins
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=20090730145728.25226.92769.stgit@dev.haskins.net \
--to=ghaskins@novell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maxk@qualcomm.com \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=rusty@rustcorp.com.au \
/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