linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] locking/static_key: improve rate limited labels
@ 2019-03-30  0:08 Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 1/3] locking/static_key: add support for deferred static branches Jakub Kicinski
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Jakub Kicinski @ 2019-03-30  0:08 UTC (permalink / raw)
  To: peterz, tglx
  Cc: ard.biesheuvel, yamada.masahiro, mingo, linux-kernel, netdev,
	oss-drivers, alexei.starovoitov, Jakub Kicinski

Hi!

This will be used to fix the static branch disabling in the TLS
code.  The net/tls/ code should be using the deferred static
branch type, because unprivileged users can flip the branch
on and off quite easily with CONFIG_TLS_DEVICE=y.

Second of all we shouldn't take the jump label locks from
the RX path, when the socket is destroyed.  This can be avoided
with some slight code refactoring in deferred static_key as
it already runs from a workqueue.

This the series (and a simple tls patch which makes use of it)
applied opening 0.5M TLS connections to localhost (just calling
setsockopt, no data exchange) goes down from 37.9s to 12.4s.

Jakub Kicinski (3):
  locking/static_key: add support for deferred static branches
  locking/static_key: factor out the fast path of static_key_slow_dec()
  locking/static_key: don't take sleeping locks in
    __static_key_slow_dec_deferred()

 include/linux/jump_label_ratelimit.h | 64 ++++++++++++++++++++++++++--
 kernel/jump_label.c                  | 64 +++++++++++++++-------------
 2 files changed, 95 insertions(+), 33 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] locking/static_key: add support for deferred static branches
  2019-03-30  0:08 [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
@ 2019-03-30  0:08 ` Jakub Kicinski
  2019-04-29  6:40   ` [tip:locking/core] locking/static_key: Add " tip-bot for Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 2/3] locking/static_key: factor out the fast path of static_key_slow_dec() Jakub Kicinski
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2019-03-30  0:08 UTC (permalink / raw)
  To: peterz, tglx
  Cc: ard.biesheuvel, yamada.masahiro, mingo, linux-kernel, netdev,
	oss-drivers, alexei.starovoitov, Jakub Kicinski, Simon Horman

Add deferred static branches.  We can't unfortunately use the
nice trick of encapsulating the entire structure in true/false
variants, because the inside has to be either struct static_key_true
or struct static_key_false.  Use defines to pass the appropriate
members to the helpers separately.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/linux/jump_label_ratelimit.h | 64 ++++++++++++++++++++++++++--
 kernel/jump_label.c                  | 17 +++++---
 2 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h
index a49f2b45b3f0..42710d5949ba 100644
--- a/include/linux/jump_label_ratelimit.h
+++ b/include/linux/jump_label_ratelimit.h
@@ -12,21 +12,79 @@ struct static_key_deferred {
 	struct delayed_work work;
 };
 
-extern void static_key_slow_dec_deferred(struct static_key_deferred *key);
-extern void static_key_deferred_flush(struct static_key_deferred *key);
+struct static_key_true_deferred {
+	struct static_key_true key;
+	unsigned long timeout;
+	struct delayed_work work;
+};
+
+struct static_key_false_deferred {
+	struct static_key_false key;
+	unsigned long timeout;
+	struct delayed_work work;
+};
+
+#define static_key_slow_dec_deferred(x)					\
+	__static_key_slow_dec_deferred(&(x)->key, &(x)->work, (x)->timeout)
+#define static_branch_slow_dec_deferred(x)				\
+	__static_key_slow_dec_deferred(&(x)->key.key, &(x)->work, (x)->timeout)
+
+#define static_key_deferred_flush(x)					\
+	__static_key_deferred_flush((x), &(x)->work)
+
+extern void
+__static_key_slow_dec_deferred(struct static_key *key,
+			       struct delayed_work *work,
+			       unsigned long timeout);
+extern void __static_key_deferred_flush(void *key, struct delayed_work *work);
 extern void
 jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
 
+extern void jump_label_update_timeout(struct work_struct *work);
+
+#define DEFINE_STATIC_KEY_DEFERRED_TRUE(name, rl)			\
+	struct static_key_true_deferred name = {			\
+		.key =		{ STATIC_KEY_INIT_TRUE },		\
+		.timeout =	(rl),					\
+		.work =	__DELAYED_WORK_INITIALIZER((name).work,		\
+						   jump_label_update_timeout, \
+						   0),			\
+	}
+
+#define DEFINE_STATIC_KEY_DEFERRED_FALSE(name, rl)			\
+	struct static_key_false_deferred name = {			\
+		.key =		{ STATIC_KEY_INIT_FALSE },		\
+		.timeout =	(rl),					\
+		.work =	__DELAYED_WORK_INITIALIZER((name).work,		\
+						   jump_label_update_timeout, \
+						   0),			\
+	}
+
+#define static_branch_deferred_inc(x)	static_branch_inc(&(x)->key)
+
 #else	/* !CONFIG_JUMP_LABEL */
 struct static_key_deferred {
 	struct static_key  key;
 };
+struct static_key_true_deferred {
+	struct static_key_true key;
+};
+struct static_key_false_deferred {
+	struct static_key_false key;
+};
+#define DEFINE_STATIC_KEY_DEFERRED_TRUE(name, rl)	\
+	struct static_key_true_deferred name = { STATIC_KEY_TRUE_INIT }
+#define DEFINE_STATIC_KEY_DEFERRED_FALSE(name, rl)	\
+	struct static_key_false_deferred name = { STATIC_KEY_FALSE_INIT }
+
+#define static_branch_slow_dec_deferred(x)	static_branch_dec(&(x)->key)
+
 static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
 {
 	STATIC_KEY_CHECK_USE(key);
 	static_key_slow_dec(&key->key);
 }
-static inline void static_key_deferred_flush(struct static_key_deferred *key)
+static inline void static_key_deferred_flush(void *key)
 {
 	STATIC_KEY_CHECK_USE(key);
 }
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index a799b1ac6b2f..73bbbaddbd9c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -244,12 +244,13 @@ static void __static_key_slow_dec(struct static_key *key,
 	cpus_read_unlock();
 }
 
-static void jump_label_update_timeout(struct work_struct *work)
+void jump_label_update_timeout(struct work_struct *work)
 {
 	struct static_key_deferred *key =
 		container_of(work, struct static_key_deferred, work.work);
 	__static_key_slow_dec(&key->key, 0, NULL);
 }
+EXPORT_SYMBOL_GPL(jump_label_update_timeout);
 
 void static_key_slow_dec(struct static_key *key)
 {
@@ -264,19 +265,21 @@ void static_key_slow_dec_cpuslocked(struct static_key *key)
 	__static_key_slow_dec_cpuslocked(key, 0, NULL);
 }
 
-void static_key_slow_dec_deferred(struct static_key_deferred *key)
+void __static_key_slow_dec_deferred(struct static_key *key,
+				    struct delayed_work *work,
+				    unsigned long timeout)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(&key->key, key->timeout, &key->work);
+	__static_key_slow_dec(key, timeout, work);
 }
-EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
+EXPORT_SYMBOL_GPL(__static_key_slow_dec_deferred);
 
-void static_key_deferred_flush(struct static_key_deferred *key)
+void __static_key_deferred_flush(void *key, struct delayed_work *work)
 {
 	STATIC_KEY_CHECK_USE(key);
-	flush_delayed_work(&key->work);
+	flush_delayed_work(work);
 }
-EXPORT_SYMBOL_GPL(static_key_deferred_flush);
+EXPORT_SYMBOL_GPL(__static_key_deferred_flush);
 
 void jump_label_rate_limit(struct static_key_deferred *key,
 		unsigned long rl)
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] locking/static_key: factor out the fast path of static_key_slow_dec()
  2019-03-30  0:08 [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 1/3] locking/static_key: add support for deferred static branches Jakub Kicinski
@ 2019-03-30  0:08 ` Jakub Kicinski
  2019-04-29  6:41   ` [tip:locking/core] locking/static_key: Factor " tip-bot for Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 3/3] locking/static_key: don't take sleeping locks in __static_key_slow_dec_deferred() Jakub Kicinski
  2019-04-01 18:21 ` [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
  3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2019-03-30  0:08 UTC (permalink / raw)
  To: peterz, tglx
  Cc: ard.biesheuvel, yamada.masahiro, mingo, linux-kernel, netdev,
	oss-drivers, alexei.starovoitov, Jakub Kicinski, Simon Horman

static_key_slow_dec() checks if the atomic enable count is larger
than 1, and if so there decrements it before taking the jump_label_lock.
Move this logic into a helper for reuse in rate limitted keys.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 kernel/jump_label.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 73bbbaddbd9c..02c3d11264dd 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -202,13 +202,13 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-static void __static_key_slow_dec_cpuslocked(struct static_key *key,
-					   unsigned long rate_limit,
-					   struct delayed_work *work)
+static bool static_key_slow_try_dec(struct static_key *key)
 {
 	int val;
 
-	lockdep_assert_cpus_held();
+	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
+	if (val == 1)
+		return false;
 
 	/*
 	 * The negative count check is valid even when a negative
@@ -217,11 +217,18 @@ static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 	 * returns is unbalanced, because all other static_key_slow_inc()
 	 * instances block while the update is in progress.
 	 */
-	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
-	if (val != 1) {
-		WARN(val < 0, "jump label: negative count!\n");
+	WARN(val < 0, "jump label: negative count!\n");
+	return true;
+}
+
+static void __static_key_slow_dec_cpuslocked(struct static_key *key,
+					   unsigned long rate_limit,
+					   struct delayed_work *work)
+{
+	lockdep_assert_cpus_held();
+
+	if (static_key_slow_try_dec(key))
 		return;
-	}
 
 	jump_label_lock();
 	if (atomic_dec_and_test(&key->enabled)) {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] locking/static_key: don't take sleeping locks in __static_key_slow_dec_deferred()
  2019-03-30  0:08 [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 1/3] locking/static_key: add support for deferred static branches Jakub Kicinski
  2019-03-30  0:08 ` [PATCH 2/3] locking/static_key: factor out the fast path of static_key_slow_dec() Jakub Kicinski
@ 2019-03-30  0:08 ` Jakub Kicinski
  2019-04-29  6:41   ` [tip:locking/core] locking/static_key: Don't " tip-bot for Jakub Kicinski
  2019-04-01 18:21 ` [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
  3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2019-03-30  0:08 UTC (permalink / raw)
  To: peterz, tglx
  Cc: ard.biesheuvel, yamada.masahiro, mingo, linux-kernel, netdev,
	oss-drivers, alexei.starovoitov, Jakub Kicinski, Simon Horman

Changing jump_label state is protected by jump_label_lock().
Rate limited static_key_slow_dec(), however, will never
directly call jump_label_update(), it will schedule a delayed
work instead.  Therefore it's unnecessary to take both the
cpus_read_lock() and jump_label_lock().

This allows static_key_slow_dec_deferred() to be called
from atomic contexts, like socket destructing in net/tls,
without the need for another indirection.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 kernel/jump_label.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 02c3d11264dd..de6efdecc70d 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -221,9 +221,7 @@ static bool static_key_slow_try_dec(struct static_key *key)
 	return true;
 }
 
-static void __static_key_slow_dec_cpuslocked(struct static_key *key,
-					   unsigned long rate_limit,
-					   struct delayed_work *work)
+static void __static_key_slow_dec_cpuslocked(struct static_key *key)
 {
 	lockdep_assert_cpus_held();
 
@@ -231,23 +229,15 @@ static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 		return;
 
 	jump_label_lock();
-	if (atomic_dec_and_test(&key->enabled)) {
-		if (rate_limit) {
-			atomic_inc(&key->enabled);
-			schedule_delayed_work(work, rate_limit);
-		} else {
-			jump_label_update(key);
-		}
-	}
+	if (atomic_dec_and_test(&key->enabled))
+		jump_label_update(key);
 	jump_label_unlock();
 }
 
-static void __static_key_slow_dec(struct static_key *key,
-				  unsigned long rate_limit,
-				  struct delayed_work *work)
+static void __static_key_slow_dec(struct static_key *key)
 {
 	cpus_read_lock();
-	__static_key_slow_dec_cpuslocked(key, rate_limit, work);
+	__static_key_slow_dec_cpuslocked(key);
 	cpus_read_unlock();
 }
 
@@ -255,21 +245,21 @@ void jump_label_update_timeout(struct work_struct *work)
 {
 	struct static_key_deferred *key =
 		container_of(work, struct static_key_deferred, work.work);
-	__static_key_slow_dec(&key->key, 0, NULL);
+	__static_key_slow_dec(&key->key);
 }
 EXPORT_SYMBOL_GPL(jump_label_update_timeout);
 
 void static_key_slow_dec(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(key, 0, NULL);
+	__static_key_slow_dec(key);
 }
 EXPORT_SYMBOL_GPL(static_key_slow_dec);
 
 void static_key_slow_dec_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec_cpuslocked(key, 0, NULL);
+	__static_key_slow_dec_cpuslocked(key);
 }
 
 void __static_key_slow_dec_deferred(struct static_key *key,
@@ -277,7 +267,11 @@ void __static_key_slow_dec_deferred(struct static_key *key,
 				    unsigned long timeout)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(key, timeout, work);
+
+	if (static_key_slow_try_dec(key))
+		return;
+
+	schedule_delayed_work(work, timeout);
 }
 EXPORT_SYMBOL_GPL(__static_key_slow_dec_deferred);
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] locking/static_key: improve rate limited labels
  2019-03-30  0:08 [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
                   ` (2 preceding siblings ...)
  2019-03-30  0:08 ` [PATCH 3/3] locking/static_key: don't take sleeping locks in __static_key_slow_dec_deferred() Jakub Kicinski
@ 2019-04-01 18:21 ` Jakub Kicinski
  2019-04-02  7:03   ` Peter Zijlstra
  2019-04-04 10:29   ` Peter Zijlstra
  3 siblings, 2 replies; 12+ messages in thread
From: Jakub Kicinski @ 2019-04-01 18:21 UTC (permalink / raw)
  To: peterz, tglx, David Miller
  Cc: ard.biesheuvel, yamada.masahiro, mingo, linux-kernel, netdev,
	oss-drivers, alexei.starovoitov

On Fri, 29 Mar 2019 17:08:51 -0700, Jakub Kicinski wrote:
> Hi!
> 
> This will be used to fix the static branch disabling in the TLS
> code.  The net/tls/ code should be using the deferred static
> branch type, because unprivileged users can flip the branch
> on and off quite easily with CONFIG_TLS_DEVICE=y.
> 
> Second of all we shouldn't take the jump label locks from
> the RX path, when the socket is destroyed.  This can be avoided
> with some slight code refactoring in deferred static_key as
> it already runs from a workqueue.
> 
> This the series (and a simple tls patch which makes use of it)
> applied opening 0.5M TLS connections to localhost (just calling
> setsockopt, no data exchange) goes down from 37.9s to 12.4s.

Once/if we get positive feedback from locking folks, would it be
possible to merge these via net-next tree alongside the patch
converting TLS to rate limited branches?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] locking/static_key: improve rate limited labels
  2019-04-01 18:21 ` [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
@ 2019-04-02  7:03   ` Peter Zijlstra
  2019-04-04 10:29   ` Peter Zijlstra
  1 sibling, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2019-04-02  7:03 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: tglx, David Miller, ard.biesheuvel, yamada.masahiro, mingo,
	linux-kernel, netdev, oss-drivers, alexei.starovoitov

On Mon, Apr 01, 2019 at 11:21:53AM -0700, Jakub Kicinski wrote:
> On Fri, 29 Mar 2019 17:08:51 -0700, Jakub Kicinski wrote:
> > Hi!
> > 
> > This will be used to fix the static branch disabling in the TLS
> > code.  The net/tls/ code should be using the deferred static
> > branch type, because unprivileged users can flip the branch
> > on and off quite easily with CONFIG_TLS_DEVICE=y.
> > 
> > Second of all we shouldn't take the jump label locks from
> > the RX path, when the socket is destroyed.  This can be avoided
> > with some slight code refactoring in deferred static_key as
> > it already runs from a workqueue.
> > 
> > This the series (and a simple tls patch which makes use of it)
> > applied opening 0.5M TLS connections to localhost (just calling
> > setsockopt, no data exchange) goes down from 37.9s to 12.4s.
> 
> Once/if we get positive feedback from locking folks, would it be
> possible to merge these via net-next tree alongside the patch
> converting TLS to rate limited branches?

Sorry, I've been ill, I'll try and have a look at these patches soon.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] locking/static_key: improve rate limited labels
  2019-04-01 18:21 ` [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
  2019-04-02  7:03   ` Peter Zijlstra
@ 2019-04-04 10:29   ` Peter Zijlstra
  2019-04-16 16:33     ` Jakub Kicinski
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2019-04-04 10:29 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: tglx, David Miller, ard.biesheuvel, yamada.masahiro, mingo,
	linux-kernel, netdev, oss-drivers, alexei.starovoitov

On Mon, Apr 01, 2019 at 11:21:53AM -0700, Jakub Kicinski wrote:
> On Fri, 29 Mar 2019 17:08:51 -0700, Jakub Kicinski wrote:
> > Hi!
> > 
> > This will be used to fix the static branch disabling in the TLS
> > code.  The net/tls/ code should be using the deferred static
> > branch type, because unprivileged users can flip the branch
> > on and off quite easily with CONFIG_TLS_DEVICE=y.
> > 
> > Second of all we shouldn't take the jump label locks from
> > the RX path, when the socket is destroyed.  This can be avoided
> > with some slight code refactoring in deferred static_key as
> > it already runs from a workqueue.
> > 
> > This the series (and a simple tls patch which makes use of it)
> > applied opening 0.5M TLS connections to localhost (just calling
> > setsockopt, no data exchange) goes down from 37.9s to 12.4s.
> 
> Once/if we get positive feedback from locking folks, would it be
> possible to merge these via net-next tree alongside the patch
> converting TLS to rate limited branches?

Looks good. If routed through the network tree because usage there:

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Otherwise let me know and I'll carry them.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] locking/static_key: improve rate limited labels
  2019-04-04 10:29   ` Peter Zijlstra
@ 2019-04-16 16:33     ` Jakub Kicinski
  2019-04-16 17:38       ` Peter Zijlstra
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2019-04-16 16:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: tglx, David Miller, ard.biesheuvel, yamada.masahiro, mingo,
	linux-kernel, netdev, oss-drivers, alexei.starovoitov

On Thu, 4 Apr 2019 12:29:02 +0200, Peter Zijlstra wrote:
> On Mon, Apr 01, 2019 at 11:21:53AM -0700, Jakub Kicinski wrote:
> > On Fri, 29 Mar 2019 17:08:51 -0700, Jakub Kicinski wrote:  
> > > Hi!
> > > 
> > > This will be used to fix the static branch disabling in the TLS
> > > code.  The net/tls/ code should be using the deferred static
> > > branch type, because unprivileged users can flip the branch
> > > on and off quite easily with CONFIG_TLS_DEVICE=y.
> > > 
> > > Second of all we shouldn't take the jump label locks from
> > > the RX path, when the socket is destroyed.  This can be avoided
> > > with some slight code refactoring in deferred static_key as
> > > it already runs from a workqueue.
> > > 
> > > This the series (and a simple tls patch which makes use of it)
> > > applied opening 0.5M TLS connections to localhost (just calling
> > > setsockopt, no data exchange) goes down from 37.9s to 12.4s.  
> > 
> > Once/if we get positive feedback from locking folks, would it be
> > possible to merge these via net-next tree alongside the patch
> > converting TLS to rate limited branches?  
> 
> Looks good. If routed through the network tree because usage there:
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Otherwise let me know and I'll carry them.

Hi Peter!  I was initially hoping that a1247d06d010
("locking/static_key: Fix false positive warnings on concurrent dec/inc")
may go into 5.1, but it's not really a regression.  It will conflict, so
the net-next route won't work.  Would you be able to carry this set
after all?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] locking/static_key: improve rate limited labels
  2019-04-16 16:33     ` Jakub Kicinski
@ 2019-04-16 17:38       ` Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2019-04-16 17:38 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: tglx, David Miller, ard.biesheuvel, yamada.masahiro, mingo,
	linux-kernel, netdev, oss-drivers, alexei.starovoitov

On Tue, Apr 16, 2019 at 09:33:15AM -0700, Jakub Kicinski wrote:
> On Thu, 4 Apr 2019 12:29:02 +0200, Peter Zijlstra wrote:
> > On Mon, Apr 01, 2019 at 11:21:53AM -0700, Jakub Kicinski wrote:
> > > On Fri, 29 Mar 2019 17:08:51 -0700, Jakub Kicinski wrote:  
> > > > Hi!
> > > > 
> > > > This will be used to fix the static branch disabling in the TLS
> > > > code.  The net/tls/ code should be using the deferred static
> > > > branch type, because unprivileged users can flip the branch
> > > > on and off quite easily with CONFIG_TLS_DEVICE=y.
> > > > 
> > > > Second of all we shouldn't take the jump label locks from
> > > > the RX path, when the socket is destroyed.  This can be avoided
> > > > with some slight code refactoring in deferred static_key as
> > > > it already runs from a workqueue.
> > > > 
> > > > This the series (and a simple tls patch which makes use of it)
> > > > applied opening 0.5M TLS connections to localhost (just calling
> > > > setsockopt, no data exchange) goes down from 37.9s to 12.4s.  
> > > 
> > > Once/if we get positive feedback from locking folks, would it be
> > > possible to merge these via net-next tree alongside the patch
> > > converting TLS to rate limited branches?  
> > 
> > Looks good. If routed through the network tree because usage there:
> > 
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > 
> > Otherwise let me know and I'll carry them.
> 
> Hi Peter!  I was initially hoping that a1247d06d010
> ("locking/static_key: Fix false positive warnings on concurrent dec/inc")
> may go into 5.1, but it's not really a regression.  It will conflict, so
> the net-next route won't work.  Would you be able to carry this set
> after all?

n/p, done!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:locking/core] locking/static_key: Add support for deferred static branches
  2019-03-30  0:08 ` [PATCH 1/3] locking/static_key: add support for deferred static branches Jakub Kicinski
@ 2019-04-29  6:40   ` tip-bot for Jakub Kicinski
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Jakub Kicinski @ 2019-04-29  6:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: akpm, will.deacon, jakub.kicinski, torvalds, paulmck, hpa, mingo,
	peterz, simon.horman, tglx, linux-kernel

Commit-ID:  ad282a8117d5048398f506f20b092c14b3b3c43f
Gitweb:     https://git.kernel.org/tip/ad282a8117d5048398f506f20b092c14b3b3c43f
Author:     Jakub Kicinski <jakub.kicinski@netronome.com>
AuthorDate: Fri, 29 Mar 2019 17:08:52 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 29 Apr 2019 08:29:20 +0200

locking/static_key: Add support for deferred static branches

Add deferred static branches.  We can't unfortunately use the
nice trick of encapsulating the entire structure in true/false
variants, because the inside has to be either struct static_key_true
or struct static_key_false.  Use defines to pass the appropriate
members to the helpers separately.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: alexei.starovoitov@gmail.com
Cc: ard.biesheuvel@linaro.org
Cc: oss-drivers@netronome.com
Cc: yamada.masahiro@socionext.com
Link: https://lkml.kernel.org/r/20190330000854.30142-2-jakub.kicinski@netronome.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/jump_label_ratelimit.h | 64 ++++++++++++++++++++++++++++++++++--
 kernel/jump_label.c                  | 17 ++++++----
 2 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h
index a49f2b45b3f0..42710d5949ba 100644
--- a/include/linux/jump_label_ratelimit.h
+++ b/include/linux/jump_label_ratelimit.h
@@ -12,21 +12,79 @@ struct static_key_deferred {
 	struct delayed_work work;
 };
 
-extern void static_key_slow_dec_deferred(struct static_key_deferred *key);
-extern void static_key_deferred_flush(struct static_key_deferred *key);
+struct static_key_true_deferred {
+	struct static_key_true key;
+	unsigned long timeout;
+	struct delayed_work work;
+};
+
+struct static_key_false_deferred {
+	struct static_key_false key;
+	unsigned long timeout;
+	struct delayed_work work;
+};
+
+#define static_key_slow_dec_deferred(x)					\
+	__static_key_slow_dec_deferred(&(x)->key, &(x)->work, (x)->timeout)
+#define static_branch_slow_dec_deferred(x)				\
+	__static_key_slow_dec_deferred(&(x)->key.key, &(x)->work, (x)->timeout)
+
+#define static_key_deferred_flush(x)					\
+	__static_key_deferred_flush((x), &(x)->work)
+
+extern void
+__static_key_slow_dec_deferred(struct static_key *key,
+			       struct delayed_work *work,
+			       unsigned long timeout);
+extern void __static_key_deferred_flush(void *key, struct delayed_work *work);
 extern void
 jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
 
+extern void jump_label_update_timeout(struct work_struct *work);
+
+#define DEFINE_STATIC_KEY_DEFERRED_TRUE(name, rl)			\
+	struct static_key_true_deferred name = {			\
+		.key =		{ STATIC_KEY_INIT_TRUE },		\
+		.timeout =	(rl),					\
+		.work =	__DELAYED_WORK_INITIALIZER((name).work,		\
+						   jump_label_update_timeout, \
+						   0),			\
+	}
+
+#define DEFINE_STATIC_KEY_DEFERRED_FALSE(name, rl)			\
+	struct static_key_false_deferred name = {			\
+		.key =		{ STATIC_KEY_INIT_FALSE },		\
+		.timeout =	(rl),					\
+		.work =	__DELAYED_WORK_INITIALIZER((name).work,		\
+						   jump_label_update_timeout, \
+						   0),			\
+	}
+
+#define static_branch_deferred_inc(x)	static_branch_inc(&(x)->key)
+
 #else	/* !CONFIG_JUMP_LABEL */
 struct static_key_deferred {
 	struct static_key  key;
 };
+struct static_key_true_deferred {
+	struct static_key_true key;
+};
+struct static_key_false_deferred {
+	struct static_key_false key;
+};
+#define DEFINE_STATIC_KEY_DEFERRED_TRUE(name, rl)	\
+	struct static_key_true_deferred name = { STATIC_KEY_TRUE_INIT }
+#define DEFINE_STATIC_KEY_DEFERRED_FALSE(name, rl)	\
+	struct static_key_false_deferred name = { STATIC_KEY_FALSE_INIT }
+
+#define static_branch_slow_dec_deferred(x)	static_branch_dec(&(x)->key)
+
 static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
 {
 	STATIC_KEY_CHECK_USE(key);
 	static_key_slow_dec(&key->key);
 }
-static inline void static_key_deferred_flush(struct static_key_deferred *key)
+static inline void static_key_deferred_flush(void *key)
 {
 	STATIC_KEY_CHECK_USE(key);
 }
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index a799b1ac6b2f..73bbbaddbd9c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -244,12 +244,13 @@ static void __static_key_slow_dec(struct static_key *key,
 	cpus_read_unlock();
 }
 
-static void jump_label_update_timeout(struct work_struct *work)
+void jump_label_update_timeout(struct work_struct *work)
 {
 	struct static_key_deferred *key =
 		container_of(work, struct static_key_deferred, work.work);
 	__static_key_slow_dec(&key->key, 0, NULL);
 }
+EXPORT_SYMBOL_GPL(jump_label_update_timeout);
 
 void static_key_slow_dec(struct static_key *key)
 {
@@ -264,19 +265,21 @@ void static_key_slow_dec_cpuslocked(struct static_key *key)
 	__static_key_slow_dec_cpuslocked(key, 0, NULL);
 }
 
-void static_key_slow_dec_deferred(struct static_key_deferred *key)
+void __static_key_slow_dec_deferred(struct static_key *key,
+				    struct delayed_work *work,
+				    unsigned long timeout)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(&key->key, key->timeout, &key->work);
+	__static_key_slow_dec(key, timeout, work);
 }
-EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
+EXPORT_SYMBOL_GPL(__static_key_slow_dec_deferred);
 
-void static_key_deferred_flush(struct static_key_deferred *key)
+void __static_key_deferred_flush(void *key, struct delayed_work *work)
 {
 	STATIC_KEY_CHECK_USE(key);
-	flush_delayed_work(&key->work);
+	flush_delayed_work(work);
 }
-EXPORT_SYMBOL_GPL(static_key_deferred_flush);
+EXPORT_SYMBOL_GPL(__static_key_deferred_flush);
 
 void jump_label_rate_limit(struct static_key_deferred *key,
 		unsigned long rl)

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:locking/core] locking/static_key: Factor out the fast path of static_key_slow_dec()
  2019-03-30  0:08 ` [PATCH 2/3] locking/static_key: factor out the fast path of static_key_slow_dec() Jakub Kicinski
@ 2019-04-29  6:41   ` tip-bot for Jakub Kicinski
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Jakub Kicinski @ 2019-04-29  6:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, mingo, linux-kernel, paulmck, simon.horman, peterz,
	will.deacon, hpa, jakub.kicinski, torvalds, akpm

Commit-ID:  b92e793bbe4a1c49dbf78d8d526561e7a7dd568a
Gitweb:     https://git.kernel.org/tip/b92e793bbe4a1c49dbf78d8d526561e7a7dd568a
Author:     Jakub Kicinski <jakub.kicinski@netronome.com>
AuthorDate: Fri, 29 Mar 2019 17:08:53 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 29 Apr 2019 08:29:21 +0200

locking/static_key: Factor out the fast path of static_key_slow_dec()

static_key_slow_dec() checks if the atomic enable count is larger
than 1, and if so there decrements it before taking the jump_label_lock.
Move this logic into a helper for reuse in rate limitted keys.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: alexei.starovoitov@gmail.com
Cc: ard.biesheuvel@linaro.org
Cc: oss-drivers@netronome.com
Cc: yamada.masahiro@socionext.com
Link: https://lkml.kernel.org/r/20190330000854.30142-3-jakub.kicinski@netronome.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/jump_label.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 73bbbaddbd9c..02c3d11264dd 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -202,13 +202,13 @@ void static_key_disable(struct static_key *key)
 }
 EXPORT_SYMBOL_GPL(static_key_disable);
 
-static void __static_key_slow_dec_cpuslocked(struct static_key *key,
-					   unsigned long rate_limit,
-					   struct delayed_work *work)
+static bool static_key_slow_try_dec(struct static_key *key)
 {
 	int val;
 
-	lockdep_assert_cpus_held();
+	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
+	if (val == 1)
+		return false;
 
 	/*
 	 * The negative count check is valid even when a negative
@@ -217,11 +217,18 @@ static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 	 * returns is unbalanced, because all other static_key_slow_inc()
 	 * instances block while the update is in progress.
 	 */
-	val = atomic_fetch_add_unless(&key->enabled, -1, 1);
-	if (val != 1) {
-		WARN(val < 0, "jump label: negative count!\n");
+	WARN(val < 0, "jump label: negative count!\n");
+	return true;
+}
+
+static void __static_key_slow_dec_cpuslocked(struct static_key *key,
+					   unsigned long rate_limit,
+					   struct delayed_work *work)
+{
+	lockdep_assert_cpus_held();
+
+	if (static_key_slow_try_dec(key))
 		return;
-	}
 
 	jump_label_lock();
 	if (atomic_dec_and_test(&key->enabled)) {

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip:locking/core] locking/static_key: Don't take sleeping locks in __static_key_slow_dec_deferred()
  2019-03-30  0:08 ` [PATCH 3/3] locking/static_key: don't take sleeping locks in __static_key_slow_dec_deferred() Jakub Kicinski
@ 2019-04-29  6:41   ` tip-bot for Jakub Kicinski
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Jakub Kicinski @ 2019-04-29  6:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, will.deacon, akpm, paulmck, peterz, simon.horman, mingo,
	linux-kernel, hpa, torvalds, jakub.kicinski

Commit-ID:  94b5f312cfb4a66055d9b688dc9ab6b297eb9dcc
Gitweb:     https://git.kernel.org/tip/94b5f312cfb4a66055d9b688dc9ab6b297eb9dcc
Author:     Jakub Kicinski <jakub.kicinski@netronome.com>
AuthorDate: Fri, 29 Mar 2019 17:08:54 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 29 Apr 2019 08:29:21 +0200

locking/static_key: Don't take sleeping locks in __static_key_slow_dec_deferred()

Changing jump_label state is protected by jump_label_lock().
Rate limited static_key_slow_dec(), however, will never
directly call jump_label_update(), it will schedule a delayed
work instead.  Therefore it's unnecessary to take both the
cpus_read_lock() and jump_label_lock().

This allows static_key_slow_dec_deferred() to be called
from atomic contexts, like socket destructing in net/tls,
without the need for another indirection.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: alexei.starovoitov@gmail.com
Cc: ard.biesheuvel@linaro.org
Cc: oss-drivers@netronome.com
Cc: yamada.masahiro@socionext.com
Link: https://lkml.kernel.org/r/20190330000854.30142-4-jakub.kicinski@netronome.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/jump_label.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 02c3d11264dd..de6efdecc70d 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -221,9 +221,7 @@ static bool static_key_slow_try_dec(struct static_key *key)
 	return true;
 }
 
-static void __static_key_slow_dec_cpuslocked(struct static_key *key,
-					   unsigned long rate_limit,
-					   struct delayed_work *work)
+static void __static_key_slow_dec_cpuslocked(struct static_key *key)
 {
 	lockdep_assert_cpus_held();
 
@@ -231,23 +229,15 @@ static void __static_key_slow_dec_cpuslocked(struct static_key *key,
 		return;
 
 	jump_label_lock();
-	if (atomic_dec_and_test(&key->enabled)) {
-		if (rate_limit) {
-			atomic_inc(&key->enabled);
-			schedule_delayed_work(work, rate_limit);
-		} else {
-			jump_label_update(key);
-		}
-	}
+	if (atomic_dec_and_test(&key->enabled))
+		jump_label_update(key);
 	jump_label_unlock();
 }
 
-static void __static_key_slow_dec(struct static_key *key,
-				  unsigned long rate_limit,
-				  struct delayed_work *work)
+static void __static_key_slow_dec(struct static_key *key)
 {
 	cpus_read_lock();
-	__static_key_slow_dec_cpuslocked(key, rate_limit, work);
+	__static_key_slow_dec_cpuslocked(key);
 	cpus_read_unlock();
 }
 
@@ -255,21 +245,21 @@ void jump_label_update_timeout(struct work_struct *work)
 {
 	struct static_key_deferred *key =
 		container_of(work, struct static_key_deferred, work.work);
-	__static_key_slow_dec(&key->key, 0, NULL);
+	__static_key_slow_dec(&key->key);
 }
 EXPORT_SYMBOL_GPL(jump_label_update_timeout);
 
 void static_key_slow_dec(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(key, 0, NULL);
+	__static_key_slow_dec(key);
 }
 EXPORT_SYMBOL_GPL(static_key_slow_dec);
 
 void static_key_slow_dec_cpuslocked(struct static_key *key)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec_cpuslocked(key, 0, NULL);
+	__static_key_slow_dec_cpuslocked(key);
 }
 
 void __static_key_slow_dec_deferred(struct static_key *key,
@@ -277,7 +267,11 @@ void __static_key_slow_dec_deferred(struct static_key *key,
 				    unsigned long timeout)
 {
 	STATIC_KEY_CHECK_USE(key);
-	__static_key_slow_dec(key, timeout, work);
+
+	if (static_key_slow_try_dec(key))
+		return;
+
+	schedule_delayed_work(work, timeout);
 }
 EXPORT_SYMBOL_GPL(__static_key_slow_dec_deferred);
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-04-29  6:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-30  0:08 [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
2019-03-30  0:08 ` [PATCH 1/3] locking/static_key: add support for deferred static branches Jakub Kicinski
2019-04-29  6:40   ` [tip:locking/core] locking/static_key: Add " tip-bot for Jakub Kicinski
2019-03-30  0:08 ` [PATCH 2/3] locking/static_key: factor out the fast path of static_key_slow_dec() Jakub Kicinski
2019-04-29  6:41   ` [tip:locking/core] locking/static_key: Factor " tip-bot for Jakub Kicinski
2019-03-30  0:08 ` [PATCH 3/3] locking/static_key: don't take sleeping locks in __static_key_slow_dec_deferred() Jakub Kicinski
2019-04-29  6:41   ` [tip:locking/core] locking/static_key: Don't " tip-bot for Jakub Kicinski
2019-04-01 18:21 ` [PATCH 0/3] locking/static_key: improve rate limited labels Jakub Kicinski
2019-04-02  7:03   ` Peter Zijlstra
2019-04-04 10:29   ` Peter Zijlstra
2019-04-16 16:33     ` Jakub Kicinski
2019-04-16 17:38       ` Peter Zijlstra

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).