[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <4cd4ed35e383e64af7e482f2d2827703ec943c75.1372457176.git.jbaron@akamai.com>
Date: Fri, 28 Jun 2013 22:30:36 +0000 (GMT)
From: jbaron@...mai.com
To: rostedt@...dmis.org, andi@...stfloor.org
Cc: linux-kernel@...r.kernel.org, mingo@...nel.org,
peterz@...radead.org
Subject: [PATCH 1/3] static_keys: Add a static_key_slow_set_true()/false() interface
As pointed out by Andi Kleen, some static key users can be racy because they
check the value of the key->enabled, and then subsequently update the branch
direction. A number of call sites have 'higher' level locking that avoids this
race, but the usage in the scheduler features does not. See:
http://lkml.indiana.edu/hypermail/linux/kernel/1304.2/01655.html
Thus, introduce a new API that does the check and set under the
'jump_label_mutex'. This should also allow to simplify call sites a bit.
Users of static keys should use either the inc/dec or the set_true/set_false
API.
Signed-off-by: Jason Baron <jbaron@...mai.com>
---
Documentation/static-keys.txt | 8 ++++++++
include/linux/jump_label.h | 30 ++++++++++++++++++++++++++++++
kernel/jump_label.c | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+)
diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index 9f5263d..4cca941 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -134,6 +134,14 @@ An example usage in the kernel is the implementation of tracepoints:
TP_CONDITION(cond)); \
}
+Keys can also be updated with the following calls:
+
+ static_key_slow_set_true(struct static_key *key);
+ static_key_slow_set_false(struct static_key *key);
+
+Users should of the API should not mix the inc/dec with usages
+of set_true/set_false. That is, users should choose one or the other.
+
Tracepoints are disabled by default, and can be placed in performance critical
pieces of the kernel. Thus, by using a static key, the tracepoints can have
absolutely minimal impact when not in use.
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 0976fc4..787ab73 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -67,6 +67,11 @@ struct static_key_deferred {
struct delayed_work work;
};
+/* for use with set_true/set_false API */
+struct static_key_boolean {
+ struct static_key key;
+};
+
# include <asm/jump_label.h>
# define HAVE_JUMP_LABEL
#endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
@@ -120,6 +125,8 @@ extern int jump_label_text_reserved(void *start, void *end);
extern void static_key_slow_inc(struct static_key *key);
extern void static_key_slow_dec(struct static_key *key);
extern void static_key_slow_dec_deferred(struct static_key_deferred *key);
+extern void static_key_slow_set_true(struct static_key_boolean *key);
+extern void static_key_slow_set_false(struct static_key_boolean *key);
extern void jump_label_apply_nops(struct module *mod);
extern void
jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
@@ -128,6 +135,10 @@ jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
{ .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
#define STATIC_KEY_INIT_FALSE ((struct static_key) \
{ .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
+#define STATIC_KEY_BOOLEAN_INIT_TRUE ((struct static_key_boolean) \
+ { .key.enabled = ATOMIC_INIT(1), .key.entries = (void *)1 })
+#define STATIC_KEY_BOOLEAN_INIT_FALSE ((struct static_key_boolean) \
+ { .key.enabled = ATOMIC_INIT(0), .key.entries = (void *)0 })
#else /* !HAVE_JUMP_LABEL */
@@ -137,6 +148,11 @@ struct static_key {
atomic_t enabled;
};
+/* for use with set_true/set_false API */
+struct static_key_boolean {
+ struct static_key key;
+};
+
static __always_inline void jump_label_init(void)
{
}
@@ -174,6 +190,16 @@ static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
static_key_slow_dec(&key->key);
}
+static inline void static_key_slow_set_true(struct static_key_boolean *key)
+{
+ atomic_set(&key->key.enabled, 1);
+}
+
+static inline void static_key_slow_set_false(struct static_key_boolean *key)
+{
+ atomic_set(&key->key.enabled, 0);
+}
+
static inline int jump_label_text_reserved(void *start, void *end)
{
return 0;
@@ -197,6 +223,10 @@ jump_label_rate_limit(struct static_key_deferred *key,
{ .enabled = ATOMIC_INIT(1) })
#define STATIC_KEY_INIT_FALSE ((struct static_key) \
{ .enabled = ATOMIC_INIT(0) })
+#define STATIC_KEY_BOOLEAN_INIT_TRUE ((struct static_key_boolean) \
+ { .key.enabled = ATOMIC_INIT(1) })
+#define STATIC_KEY_BOOLEAN_INIT_FALSE ((struct static_key_boolean) \
+ { .key.enabled = ATOMIC_INIT(0) })
#endif /* HAVE_JUMP_LABEL */
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 60f48fa..2234a4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -120,6 +120,46 @@ void jump_label_rate_limit(struct static_key_deferred *key,
}
EXPORT_SYMBOL_GPL(jump_label_rate_limit);
+void static_key_slow_set_true(struct static_key_boolean *key_boolean)
+{
+ struct static_key *key = (struct static_key *)key_boolean;
+ int enabled;
+
+ jump_label_lock();
+ enabled = atomic_read(&key->enabled);
+ if (enabled == 1)
+ goto out;
+ WARN(enabled != 0, "%s: key->enabled: %d\n", __func__, enabled);
+ if (!jump_label_get_branch_default(key))
+ jump_label_update(key, JUMP_LABEL_ENABLE);
+ else
+ jump_label_update(key, JUMP_LABEL_DISABLE);
+ atomic_set(&key->enabled, 1);
+out:
+ jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_slow_set_true);
+
+void static_key_slow_set_false(struct static_key_boolean *key_boolean)
+{
+ struct static_key *key = (struct static_key *)key_boolean;
+ int enabled;
+
+ jump_label_lock();
+ enabled = atomic_read(&key->enabled);
+ if (enabled == 0)
+ goto out;
+ WARN(enabled != 1, "%s: key->enabled: %d\n", __func__, enabled);
+ if (!jump_label_get_branch_default(key))
+ jump_label_update(key, JUMP_LABEL_DISABLE);
+ else
+ jump_label_update(key, JUMP_LABEL_ENABLE);
+ atomic_set(&key->enabled, 0);
+out:
+ jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_slow_set_false);
+
static int addr_conflict(struct jump_entry *entry, void *start, void *end)
{
if (entry->code <= (unsigned long)end &&
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists