Any use of key->enabled (that is static_key_enabled and static_key_count)
outside jump_label_lock should handle its own serialization. Notably
among those that don't do so is static_key_enable/disable itself. Once
those are fixed (patch 1), they can be used to fix the actual offenders in
net/ (patch 2).
Patch 3 is not fixing anything, but the function cannot be used correctly
from any place other than cpuset.c itself so it is moved there.
Based on discussion with Peter Zijlstra.
Paolo
Paolo Bonzini (3):
jump_labels: fix concurrent static_key_enable/disable()
jump_labels: do not use unserialized static_key_enabled
cpuset: make nr_cpusets private
Documentation/static-keys.txt | 5 ++++
include/linux/cpuset.h | 6 -----
include/linux/jump_label.h | 22 ++++++++--------
kernel/cgroup/cpuset.c | 7 +++++
kernel/jump_label.c | 59 +++++++++++++++++++++++++++----------------
net/ipv4/udp.c | 3 +--
net/ipv6/udp.c | 3 +--
7 files changed, 63 insertions(+), 42 deletions(-)
--
1.8.3.1
static_key_enable/disable are trying to cap the static key count to
0/1. However, their use of key->enabled is outside jump_label_lock
so they do not really ensure that.
Rewrite them to do a quick check for an already enabled (respectively,
already disabled), and then recheck under the jump label lock. Unlike
static_key_slow_inc/dec, a failed check under the jump label lock does
not modify key->enabled.
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
include/linux/jump_label.h | 22 +++++++++--------
kernel/jump_label.c | 59 +++++++++++++++++++++++++++++-----------------
2 files changed, 49 insertions(+), 32 deletions(-)
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 2afd74b9d844..740a42ea7f7f 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -234,22 +234,24 @@ static inline int jump_label_apply_nops(struct module *mod)
static inline void static_key_enable(struct static_key *key)
{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
+ STATIC_KEY_CHECK_USE();
- if (!count)
- static_key_slow_inc(key);
+ if (atomic_read(&key->enabled) != 0) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ return;
+ }
+ atomic_set(&key->enabled, 1);
}
static inline void static_key_disable(struct static_key *key)
{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
+ STATIC_KEY_CHECK_USE();
- if (count)
- static_key_slow_dec(key);
+ if (atomic_read(&key->enabled) != 1) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ return;
+ }
+ atomic_set(&key->enabled, 0);
}
#define STATIC_KEY_INIT_TRUE { .enabled = ATOMIC_INIT(1) }
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 6c9cb208ac48..04a574fd5844 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -78,28 +78,6 @@ int static_key_count(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_count);
-void static_key_enable(struct static_key *key)
-{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
-
- if (!count)
- static_key_slow_inc(key);
-}
-EXPORT_SYMBOL_GPL(static_key_enable);
-
-void static_key_disable(struct static_key *key)
-{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
-
- if (count)
- static_key_slow_dec(key);
-}
-EXPORT_SYMBOL_GPL(static_key_disable);
-
void static_key_slow_inc(struct static_key *key)
{
int v, v1;
@@ -136,6 +114,43 @@ void static_key_slow_inc(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_slow_inc);
+void static_key_enable(struct static_key *key)
+{
+ STATIC_KEY_CHECK_USE();
+ if (atomic_read(&key->enabled) > 0) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ return;
+ }
+
+ cpus_read_lock();
+ jump_label_lock();
+ if (atomic_read(&key->enabled) == 0) {
+ atomic_set(&key->enabled, -1);
+ jump_label_update(key);
+ atomic_set(&key->enabled, 1);
+ }
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_enable);
+
+void static_key_disable(struct static_key *key)
+{
+ STATIC_KEY_CHECK_USE();
+ if (atomic_read(&key->enabled) != 1) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ return;
+ }
+
+ cpus_read_lock();
+ jump_label_lock();
+ if (atomic_cmpxchg(&key->enabled, 1, 0))
+ jump_label_update(key);
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_disable);
+
static void __static_key_slow_dec(struct static_key *key,
unsigned long rate_limit, struct delayed_work *work)
{
--
1.8.3.1
Any use of key->enabled (that is static_key_enabled and static_key_count)
outside jump_label_lock should handle its own serialization. In the case
of cpusets_enabled_key, the key is always incremented/decremented under
cpuset_mutex, and hence the same rule applies to nr_cpusets. The rule
*is* respected currently, but the mutex is static so nr_cpusets should
be static too.
Cc: Peter Zijlstra <[email protected]>
Cc: Zefan Li <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
include/linux/cpuset.h | 6 ------
kernel/cgroup/cpuset.c | 7 +++++++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 119a3f9604b0..cedcc910b7a7 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -24,12 +24,6 @@ static inline bool cpusets_enabled(void)
return static_branch_unlikely(&cpusets_enabled_key);
}
-static inline int nr_cpusets(void)
-{
- /* jump label reference count + the top-level cpuset */
- return static_key_count(&cpusets_enabled_key.key) + 1;
-}
-
static inline void cpuset_inc(void)
{
static_branch_inc(&cpusets_enabled_key);
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index ae643412948a..f65db17e9e23 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -576,6 +576,13 @@ static void update_domain_attr_tree(struct sched_domain_attr *dattr,
rcu_read_unlock();
}
+/* Must be called with cpuset_mutex held. */
+static inline int nr_cpusets(void)
+{
+ /* jump label reference count + the top-level cpuset */
+ return static_key_count(&cpusets_enabled_key.key) + 1;
+}
+
/*
* generate_sched_domains()
*
--
1.8.3.1
Any use of key->enabled (that is static_key_enabled and static_key_count)
outside jump_label_lock should handle its own serialization. The only
two that are not doing so are the UDP encapsulation static keys. Change
them to use static_key_enable, which now correctly tests key->enabled under
the jump label lock.
Cc: Peter Zijlstra <[email protected]>
Cc: Eric Dumazet <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
Documentation/static-keys.txt | 5 +++++
net/ipv4/udp.c | 3 +--
net/ipv6/udp.c | 3 +--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index ef419fd0897f..181998852961 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -142,6 +142,11 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
key is initialized false, a 'static_branch_inc()', will change the branch to
true. And then a 'static_branch_dec()', will again make the branch false.
+The state and the reference count can be retrieved with 'static_key_enabled()'
+and 'static_key_count()'. In general, if you use these functions, they
+should be protected with the same mutex used around the enable/disable
+or increment/decrement function.
+
Where an array of keys is required, it can be defined as:
DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 1d6219bf2d6b..74b7810df9fc 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1644,8 +1644,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
static struct static_key udp_encap_needed __read_mostly;
void udp_encap_enable(void)
{
- if (!static_key_enabled(&udp_encap_needed))
- static_key_slow_inc(&udp_encap_needed);
+ static_key_enable(&udp_encap_needed);
}
EXPORT_SYMBOL(udp_encap_enable);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 75703fda23e7..1a96233622b7 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -564,8 +564,7 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
static struct static_key udpv6_encap_needed __read_mostly;
void udpv6_encap_enable(void)
{
- if (!static_key_enabled(&udpv6_encap_needed))
- static_key_slow_inc(&udpv6_encap_needed);
+ static_key_enable(&udpv6_encap_needed);
}
EXPORT_SYMBOL(udpv6_encap_enable);
--
1.8.3.1
Thanks for doing these patches, I hadn't come around to them yet.
On Tue, Aug 01, 2017 at 05:24:04PM +0200, Paolo Bonzini wrote:
>
> +void static_key_enable(struct static_key *key)
> +{
> + STATIC_KEY_CHECK_USE();
> + if (atomic_read(&key->enabled) > 0) {
> + WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
> + return;
> + }
> +
> + cpus_read_lock();
> + jump_label_lock();
> + if (atomic_read(&key->enabled) == 0) {
> + atomic_set(&key->enabled, -1);
> + jump_label_update(key);
As per the previous discussion, should I do a patch adding barriers here
(or using atomic_set_release()) such that we close the window where a
concurrent inc/enable sees 1 but not all text changes?
> + atomic_set(&key->enabled, 1);
> + }
> + jump_label_unlock();
> + cpus_read_unlock();
> +}
> +EXPORT_SYMBOL_GPL(static_key_enable);
On 01/08/2017 18:45, Peter Zijlstra wrote:
>
> Thanks for doing these patches, I hadn't come around to them yet.
>
> On Tue, Aug 01, 2017 at 05:24:04PM +0200, Paolo Bonzini wrote:
>>
>> +void static_key_enable(struct static_key *key)
>> +{
>> + STATIC_KEY_CHECK_USE();
>> + if (atomic_read(&key->enabled) > 0) {
>> + WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
>> + return;
>> + }
>> +
>> + cpus_read_lock();
>> + jump_label_lock();
>> + if (atomic_read(&key->enabled) == 0) {
>> + atomic_set(&key->enabled, -1);
>> + jump_label_update(key);
>
> As per the previous discussion, should I do a patch adding barriers here
> (or using atomic_set_release()) such that we close the window where a
> concurrent inc/enable sees 1 but not all text changes?
Sure, and that applies to static_key_slow_inc as well.
Paolo
>> + atomic_set(&key->enabled, 1);
>> + }
>> + jump_label_unlock();
>> + cpus_read_unlock();
>> +}
>> +EXPORT_SYMBOL_GPL(static_key_enable);
On Tue, 2017-08-01 at 17:24 +0200, Paolo Bonzini wrote:
> Any use of key->enabled (that is static_key_enabled and static_key_count)
> outside jump_label_lock should handle its own serialization. The only
> two that are not doing so are the UDP encapsulation static keys. Change
> them to use static_key_enable, which now correctly tests key->enabled under
> the jump label lock.
>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> Documentation/static-keys.txt | 5 +++++
> net/ipv4/udp.c | 3 +--
> net/ipv6/udp.c | 3 +--
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
> index ef419fd0897f..181998852961 100644
> --- a/Documentation/static-keys.txt
> +++ b/Documentation/static-keys.txt
> @@ -142,6 +142,11 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
> key is initialized false, a 'static_branch_inc()', will change the branch to
> true. And then a 'static_branch_dec()', will again make the branch false.
>
> +The state and the reference count can be retrieved with 'static_key_enabled()'
> +and 'static_key_count()'. In general, if you use these functions, they
> +should be protected with the same mutex used around the enable/disable
> +or increment/decrement function.
> +
> Where an array of keys is required, it can be defined as:
>
> DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 1d6219bf2d6b..74b7810df9fc 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1644,8 +1644,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> static struct static_key udp_encap_needed __read_mostly;
> void udp_encap_enable(void)
> {
> - if (!static_key_enabled(&udp_encap_needed))
> - static_key_slow_inc(&udp_encap_needed);
> + static_key_enable(&udp_encap_needed);
> }
Looks good to me, but static_key_enable() is not serialized either ?
I suspect you should have CCed me on patch 1/3 :)
On 2017/8/1 23:24, Paolo Bonzini wrote:
> Any use of key->enabled (that is static_key_enabled and static_key_count)
> outside jump_label_lock should handle its own serialization. In the case
> of cpusets_enabled_key, the key is always incremented/decremented under
> cpuset_mutex, and hence the same rule applies to nr_cpusets. The rule
> *is* respected currently, but the mutex is static so nr_cpusets should
> be static too.
>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Zefan Li <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
Acked-by: Zefan Li <[email protected]>
> ---
> include/linux/cpuset.h | 6 ------
> kernel/cgroup/cpuset.c | 7 +++++++
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index 119a3f9604b0..cedcc910b7a7 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -24,12 +24,6 @@ static inline bool cpusets_enabled(void)
> return static_branch_unlikely(&cpusets_enabled_key);
> }
>
> -static inline int nr_cpusets(void)
> -{
> - /* jump label reference count + the top-level cpuset */
> - return static_key_count(&cpusets_enabled_key.key) + 1;
> -}
> -
> static inline void cpuset_inc(void)
> {
> static_branch_inc(&cpusets_enabled_key);
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index ae643412948a..f65db17e9e23 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -576,6 +576,13 @@ static void update_domain_attr_tree(struct sched_domain_attr *dattr,
> rcu_read_unlock();
> }
>
> +/* Must be called with cpuset_mutex held. */
> +static inline int nr_cpusets(void)
> +{
> + /* jump label reference count + the top-level cpuset */
> + return static_key_count(&cpusets_enabled_key.key) + 1;
> +}
> +
> /*
> * generate_sched_domains()
> *
>
----- Original Message -----
> From: "Eric Dumazet" <[email protected]>
> To: "Paolo Bonzini" <[email protected]>
> Cc: [email protected], "Peter Zijlstra" <[email protected]>
> Sent: Tuesday, August 1, 2017 10:52:14 PM
> Subject: Re: [PATCH 2/3] jump_labels: do not use unserialized static_key_enabled
>
> On Tue, 2017-08-01 at 17:24 +0200, Paolo Bonzini wrote:
> > Any use of key->enabled (that is static_key_enabled and static_key_count)
> > outside jump_label_lock should handle its own serialization. The only
> > two that are not doing so are the UDP encapsulation static keys. Change
> > them to use static_key_enable, which now correctly tests key->enabled under
> > the jump label lock.
> >
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index 1d6219bf2d6b..74b7810df9fc 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -1644,8 +1644,7 @@ static int __udp_queue_rcv_skb(struct sock *sk,
> > struct sk_buff *skb)
> > static struct static_key udp_encap_needed __read_mostly;
> > void udp_encap_enable(void)
> > {
> > - if (!static_key_enabled(&udp_encap_needed))
> > - static_key_slow_inc(&udp_encap_needed);
> > + static_key_enable(&udp_encap_needed);
> > }
>
> Looks good to me, but static_key_enable() is not serialized either ?
>
> I suspect you should have CCed me on patch 1/3 :)
Yes, indeed... That's the "now correctly test" in the commit message.
What a difference one word makes. :)
You can find 1/3 at https://patchwork.kernel.org/patch/9874845/.
Paolo
Commit-ID: be040bea9085a9c2b1700c9e60888777baeb96d5
Gitweb: http://git.kernel.org/tip/be040bea9085a9c2b1700c9e60888777baeb96d5
Author: Paolo Bonzini <[email protected]>
AuthorDate: Tue, 1 Aug 2017 17:24:06 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 10 Aug 2017 12:28:57 +0200
cpuset: Make nr_cpusets private
Any use of key->enabled (that is static_key_enabled and static_key_count)
outside jump_label_lock should handle its own serialization. In the case
of cpusets_enabled_key, the key is always incremented/decremented under
cpuset_mutex, and hence the same rule applies to nr_cpusets. The rule
*is* respected currently, but the mutex is static so nr_cpusets should
be static too.
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Zefan Li <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/cpuset.h | 6 ------
kernel/cgroup/cpuset.c | 7 +++++++
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 898cfe2..e74655d 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -37,12 +37,6 @@ static inline bool cpusets_enabled(void)
return static_branch_unlikely(&cpusets_enabled_key);
}
-static inline int nr_cpusets(void)
-{
- /* jump label reference count + the top-level cpuset */
- return static_key_count(&cpusets_enabled_key.key) + 1;
-}
-
static inline void cpuset_inc(void)
{
static_branch_inc(&cpusets_pre_enable_key);
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 8d51516..9ed6a05 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -577,6 +577,13 @@ static void update_domain_attr_tree(struct sched_domain_attr *dattr,
rcu_read_unlock();
}
+/* Must be called with cpuset_mutex held. */
+static inline int nr_cpusets(void)
+{
+ /* jump label reference count + the top-level cpuset */
+ return static_key_count(&cpusets_enabled_key.key) + 1;
+}
+
/*
* generate_sched_domains()
*
Commit-ID: 1dbb6704de91b169a58d0c8221624afd6a95cfc7
Gitweb: http://git.kernel.org/tip/1dbb6704de91b169a58d0c8221624afd6a95cfc7
Author: Paolo Bonzini <[email protected]>
AuthorDate: Tue, 1 Aug 2017 17:24:04 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 10 Aug 2017 12:28:56 +0200
jump_label: Fix concurrent static_key_enable/disable()
static_key_enable/disable are trying to cap the static key count to
0/1. However, their use of key->enabled is outside jump_label_lock
so they do not really ensure that.
Rewrite them to do a quick check for an already enabled (respectively,
already disabled), and then recheck under the jump label lock. Unlike
static_key_slow_inc/dec, a failed check under the jump label lock does
not modify key->enabled.
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
include/linux/jump_label.h | 22 +++++++++--------
kernel/jump_label.c | 59 +++++++++++++++++++++++++++++-----------------
2 files changed, 49 insertions(+), 32 deletions(-)
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 2afd74b..740a42e 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -234,22 +234,24 @@ static inline int jump_label_apply_nops(struct module *mod)
static inline void static_key_enable(struct static_key *key)
{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
+ STATIC_KEY_CHECK_USE();
- if (!count)
- static_key_slow_inc(key);
+ if (atomic_read(&key->enabled) != 0) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ return;
+ }
+ atomic_set(&key->enabled, 1);
}
static inline void static_key_disable(struct static_key *key)
{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
+ STATIC_KEY_CHECK_USE();
- if (count)
- static_key_slow_dec(key);
+ if (atomic_read(&key->enabled) != 1) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ return;
+ }
+ atomic_set(&key->enabled, 0);
}
#define STATIC_KEY_INIT_TRUE { .enabled = ATOMIC_INIT(1) }
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d11c506..833eeca 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -79,28 +79,6 @@ int static_key_count(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_count);
-void static_key_enable(struct static_key *key)
-{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
-
- if (!count)
- static_key_slow_inc(key);
-}
-EXPORT_SYMBOL_GPL(static_key_enable);
-
-void static_key_disable(struct static_key *key)
-{
- int count = static_key_count(key);
-
- WARN_ON_ONCE(count < 0 || count > 1);
-
- if (count)
- static_key_slow_dec(key);
-}
-EXPORT_SYMBOL_GPL(static_key_disable);
-
void static_key_slow_inc(struct static_key *key)
{
int v, v1;
@@ -139,6 +117,43 @@ void static_key_slow_inc(struct static_key *key)
}
EXPORT_SYMBOL_GPL(static_key_slow_inc);
+void static_key_enable(struct static_key *key)
+{
+ STATIC_KEY_CHECK_USE();
+ if (atomic_read(&key->enabled) > 0) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ return;
+ }
+
+ cpus_read_lock();
+ jump_label_lock();
+ if (atomic_read(&key->enabled) == 0) {
+ atomic_set(&key->enabled, -1);
+ jump_label_update(key);
+ atomic_set(&key->enabled, 1);
+ }
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_enable);
+
+void static_key_disable(struct static_key *key)
+{
+ STATIC_KEY_CHECK_USE();
+ if (atomic_read(&key->enabled) != 1) {
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ return;
+ }
+
+ cpus_read_lock();
+ jump_label_lock();
+ if (atomic_cmpxchg(&key->enabled, 1, 0))
+ jump_label_update(key);
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_disable);
+
static void __static_key_slow_dec(struct static_key *key,
unsigned long rate_limit, struct delayed_work *work)
{
Commit-ID: 7a34bcb8b272b1300f0125c93a54f0c98812acdd
Gitweb: http://git.kernel.org/tip/7a34bcb8b272b1300f0125c93a54f0c98812acdd
Author: Paolo Bonzini <[email protected]>
AuthorDate: Tue, 1 Aug 2017 17:24:05 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 10 Aug 2017 12:28:56 +0200
jump_label: Do not use unserialized static_key_enabled()
Any use of key->enabled (that is static_key_enabled and static_key_count)
outside jump_label_lock should handle its own serialization. The only
two that are not doing so are the UDP encapsulation static keys. Change
them to use static_key_enable, which now correctly tests key->enabled under
the jump label lock.
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
Documentation/static-keys.txt | 5 +++++
net/ipv4/udp.c | 3 +--
net/ipv6/udp.c | 3 +--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index b83dfa1..870b4be 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -149,6 +149,11 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
key is initialized false, a 'static_branch_inc()', will change the branch to
true. And then a 'static_branch_dec()', will again make the branch false.
+The state and the reference count can be retrieved with 'static_key_enabled()'
+and 'static_key_count()'. In general, if you use these functions, they
+should be protected with the same mutex used around the enable/disable
+or increment/decrement function.
+
Where an array of keys is required, it can be defined as::
DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e6276fa..3037339 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1809,8 +1809,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
static struct static_key udp_encap_needed __read_mostly;
void udp_encap_enable(void)
{
- if (!static_key_enabled(&udp_encap_needed))
- static_key_slow_inc(&udp_encap_needed);
+ static_key_enable(&udp_encap_needed);
}
EXPORT_SYMBOL(udp_encap_enable);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 578142b..96d2407 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -574,8 +574,7 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
static struct static_key udpv6_encap_needed __read_mostly;
void udpv6_encap_enable(void)
{
- if (!static_key_enabled(&udpv6_encap_needed))
- static_key_slow_inc(&udpv6_encap_needed);
+ static_key_enable(&udpv6_encap_needed);
}
EXPORT_SYMBOL(udpv6_encap_enable);