[PATCH 2/3] sched: fix static keys race in sched_feat

From: jbaron
Date: Fri Jun 28 2013 - 18:30:49 EST


As pointed out by Andi Kleen, thue usage of static keys can be racy in
sched_feat_disable vs. sched_feat_enable(). Currently, we first check the
value of keys->enabled, and subsequently update the branch direction. This,
can be racy and can potentially leave the keys in an inconsistent state.

Fix the race by using static_key_slow_set_true()/false(), which does the the
check and set using the jump_label_mutex.

Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>
---
kernel/sched/core.c | 12 +++++-------
kernel/sched/sched.h | 10 +++++-----
2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e8b3350..2ebf82e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -165,13 +165,13 @@ static int sched_feat_show(struct seq_file *m, void *v)

#ifdef HAVE_JUMP_LABEL

-#define jump_label_key__true STATIC_KEY_INIT_TRUE
-#define jump_label_key__false STATIC_KEY_INIT_FALSE
+#define jump_label_key__true STATIC_KEY_BOOLEAN_INIT_TRUE
+#define jump_label_key__false STATIC_KEY_BOOLEAN_INIT_FALSE

#define SCHED_FEAT(name, enabled) \
jump_label_key__##enabled ,

-struct static_key sched_feat_keys[__SCHED_FEAT_NR] = {
+struct static_key_boolean sched_feat_keys[__SCHED_FEAT_NR] = {
#include "features.h"
};

@@ -179,14 +179,12 @@ struct static_key sched_feat_keys[__SCHED_FEAT_NR] = {

static void sched_feat_disable(int i)
{
- if (static_key_enabled(&sched_feat_keys[i]))
- static_key_slow_dec(&sched_feat_keys[i]);
+ static_key_slow_set_false(&sched_feat_keys[i]);
}

static void sched_feat_enable(int i)
{
- if (!static_key_enabled(&sched_feat_keys[i]))
- static_key_slow_inc(&sched_feat_keys[i]);
+ static_key_slow_set_true(&sched_feat_keys[i]);
}
#else
static void sched_feat_disable(int i) { };
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ce39224..88712dc 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -742,17 +742,17 @@ static __always_inline bool static_branch__false(struct static_key *key)
return static_key_false(key); /* Out of line branch. */
}

-#define SCHED_FEAT(name, enabled) \
-static __always_inline bool static_branch_##name(struct static_key *key) \
-{ \
- return static_branch__##enabled(key); \
+#define SCHED_FEAT(name, enabled) \
+static __always_inline bool static_branch_##name(struct static_key_boolean *bool_key)\
+{ \
+ return static_branch__##enabled(&bool_key->key); \
}

#include "features.h"

#undef SCHED_FEAT

-extern struct static_key sched_feat_keys[__SCHED_FEAT_NR];
+extern struct static_key_boolean sched_feat_keys[__SCHED_FEAT_NR];
#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
#else /* !(SCHED_DEBUG && HAVE_JUMP_LABEL) */
#define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/