* [PATCH] genirq: change force_irqthreads from bool test to static key
@ 2021-06-02 18:03 Tanner Love
2021-06-02 21:05 ` Kees Cook
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Tanner Love @ 2021-06-02 18:03 UTC (permalink / raw)
To: linux-kernel
Cc: David S . Miller, Eric Dumazet, Willem de Bruijn, Thomas Gleixner,
Peter Zijlstra, Kees Cook, Greg Kroah-Hartman, John Garry,
Barry Song, Romain Perier, Tanner Love
From: Tanner Love <tannerlove@google.com>
With CONFIG_IRQ_FORCED_THREADING=y, testing the bool force_irqthreads
could incur a cache line miss in invoke_softirq().
Replace the test with a static key to avoid the potential cache miss.
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Tanner Love <tannerlove@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
drivers/ide/ide-iops.c | 7 +++----
include/linux/interrupt.h | 4 +++-
kernel/irq/manage.c | 6 +++---
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index f2be127ee96e..f86cdb8451e6 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -109,7 +109,6 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
ide_hwif_t *hwif = drive->hwif;
const struct ide_tp_ops *tp_ops = hwif->tp_ops;
unsigned long flags;
- bool irqs_threaded = force_irqthreads;
int i;
u8 stat;
@@ -117,7 +116,7 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
stat = tp_ops->read_status(hwif);
if (stat & ATA_BUSY) {
- if (!irqs_threaded) {
+ if (!force_irqthreads) {
local_save_flags(flags);
local_irq_enable_in_hardirq();
}
@@ -133,13 +132,13 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
if ((stat & ATA_BUSY) == 0)
break;
- if (!irqs_threaded)
+ if (!force_irqthreads)
local_irq_restore(flags);
*rstat = stat;
return -EBUSY;
}
}
- if (!irqs_threaded)
+ if (!force_irqthreads)
local_irq_restore(flags);
}
/*
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 4777850a6dc7..9e676c351f23 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -13,6 +13,7 @@
#include <linux/hrtimer.h>
#include <linux/kref.h>
#include <linux/workqueue.h>
+#include <linux/jump_label.h>
#include <linux/atomic.h>
#include <asm/ptrace.h>
@@ -504,7 +505,8 @@ extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
# ifdef CONFIG_PREEMPT_RT
# define force_irqthreads (true)
# else
-extern bool force_irqthreads;
+DECLARE_STATIC_KEY_FALSE(force_irqthreads_key);
+# define force_irqthreads (static_branch_unlikely(&force_irqthreads_key))
# endif
#else
#define force_irqthreads (0)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 4c14356543d9..395945e54929 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -25,12 +25,12 @@
#include "internals.h"
#if defined(CONFIG_IRQ_FORCED_THREADING) && !defined(CONFIG_PREEMPT_RT)
-__read_mostly bool force_irqthreads;
-EXPORT_SYMBOL_GPL(force_irqthreads);
+DEFINE_STATIC_KEY_FALSE(force_irqthreads_key);
+EXPORT_SYMBOL_GPL(force_irqthreads_key);
static int __init setup_forced_irqthreads(char *arg)
{
- force_irqthreads = true;
+ static_branch_enable(&force_irqthreads_key);
return 0;
}
early_param("threadirqs", setup_forced_irqthreads);
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] genirq: change force_irqthreads from bool test to static key
2021-06-02 18:03 [PATCH] genirq: change force_irqthreads from bool test to static key Tanner Love
@ 2021-06-02 21:05 ` Kees Cook
2021-08-10 14:12 ` [tip: irq/core] genirq: Change force_irqthreads to a " tip-bot2 for Tanner Love
2021-08-10 20:52 ` tip-bot2 for Tanner Love
2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2021-06-02 21:05 UTC (permalink / raw)
To: Tanner Love
Cc: linux-kernel, David S . Miller, Eric Dumazet, Willem de Bruijn,
Thomas Gleixner, Peter Zijlstra, Greg Kroah-Hartman, John Garry,
Barry Song, Romain Perier, Tanner Love
On Wed, Jun 02, 2021 at 02:03:38PM -0400, Tanner Love wrote:
> From: Tanner Love <tannerlove@google.com>
>
> With CONFIG_IRQ_FORCED_THREADING=y, testing the bool force_irqthreads
> could incur a cache line miss in invoke_softirq().
>
> Replace the test with a static key to avoid the potential cache miss.
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Tanner Love <tannerlove@google.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> ---
> drivers/ide/ide-iops.c | 7 +++----
> include/linux/interrupt.h | 4 +++-
> kernel/irq/manage.c | 6 +++---
> 3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index f2be127ee96e..f86cdb8451e6 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -109,7 +109,6 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
> ide_hwif_t *hwif = drive->hwif;
> const struct ide_tp_ops *tp_ops = hwif->tp_ops;
> unsigned long flags;
> - bool irqs_threaded = force_irqthreads;
> int i;
> u8 stat;
>
> @@ -117,7 +116,7 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
> stat = tp_ops->read_status(hwif);
>
> if (stat & ATA_BUSY) {
> - if (!irqs_threaded) {
> + if (!force_irqthreads) {
> local_save_flags(flags);
> local_irq_enable_in_hardirq();
> }
> @@ -133,13 +132,13 @@ int __ide_wait_stat(ide_drive_t *drive, u8 good, u8 bad,
> if ((stat & ATA_BUSY) == 0)
> break;
>
> - if (!irqs_threaded)
> + if (!force_irqthreads)
> local_irq_restore(flags);
> *rstat = stat;
> return -EBUSY;
> }
> }
> - if (!irqs_threaded)
> + if (!force_irqthreads)
> local_irq_restore(flags);
> }
> /*
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 4777850a6dc7..9e676c351f23 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -13,6 +13,7 @@
> #include <linux/hrtimer.h>
> #include <linux/kref.h>
> #include <linux/workqueue.h>
> +#include <linux/jump_label.h>
>
> #include <linux/atomic.h>
> #include <asm/ptrace.h>
> @@ -504,7 +505,8 @@ extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
> # ifdef CONFIG_PREEMPT_RT
> # define force_irqthreads (true)
> # else
> -extern bool force_irqthreads;
> +DECLARE_STATIC_KEY_FALSE(force_irqthreads_key);
> +# define force_irqthreads (static_branch_unlikely(&force_irqthreads_key))
I like this idiom, though it did make me look twice because I thought
you were missing the static_branch...() calls above. ;)
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> # endif
> #else
> #define force_irqthreads (0)
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 4c14356543d9..395945e54929 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -25,12 +25,12 @@
> #include "internals.h"
>
> #if defined(CONFIG_IRQ_FORCED_THREADING) && !defined(CONFIG_PREEMPT_RT)
> -__read_mostly bool force_irqthreads;
> -EXPORT_SYMBOL_GPL(force_irqthreads);
> +DEFINE_STATIC_KEY_FALSE(force_irqthreads_key);
> +EXPORT_SYMBOL_GPL(force_irqthreads_key);
>
> static int __init setup_forced_irqthreads(char *arg)
> {
> - force_irqthreads = true;
> + static_branch_enable(&force_irqthreads_key);
> return 0;
> }
> early_param("threadirqs", setup_forced_irqthreads);
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip: irq/core] genirq: Change force_irqthreads to a static key
2021-06-02 18:03 [PATCH] genirq: change force_irqthreads from bool test to static key Tanner Love
2021-06-02 21:05 ` Kees Cook
@ 2021-08-10 14:12 ` tip-bot2 for Tanner Love
2021-08-10 20:52 ` tip-bot2 for Tanner Love
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Tanner Love @ 2021-08-10 14:12 UTC (permalink / raw)
To: linux-tip-commits
Cc: Eric Dumazet, Tanner Love, Thomas Gleixner, Kees Cook, x86,
linux-kernel, maz
The following commit has been merged into the irq/core branch of tip:
Commit-ID: af5b7fe6bb77ac775d446e2f25f013d5df551e9a
Gitweb: https://git.kernel.org/tip/af5b7fe6bb77ac775d446e2f25f013d5df551e9a
Author: Tanner Love <tannerlove@google.com>
AuthorDate: Wed, 02 Jun 2021 14:03:38 -04:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 10 Aug 2021 16:06:55 +02:00
genirq: Change force_irqthreads to a static key
With CONFIG_IRQ_FORCED_THREADING=y, testing the boolean force_irqthreads
could incur a cache line miss in invoke_softirq() and other places.
Replace the test with a static key to avoid the potential cache miss.
[ tglx: Dropped the IDE part, removed the export and updated blk-mq ]
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Tanner Love <tannerlove@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20210602180338.3324213-1-tannerlove.kernel@gmail.com
---
block/blk-mq.c | 2 +-
include/linux/interrupt.h | 8 +++++---
kernel/irq/manage.c | 9 ++++-----
kernel/softirq.c | 2 +-
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c4ac51..572d8ab 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -606,7 +606,7 @@ static inline bool blk_mq_complete_need_ipi(struct request *rq)
* This is probably worse than completing the request on a different
* cache domain.
*/
- if (force_irqthreads)
+ if (force_irqthreads())
return false;
/* same CPU or cache domain? Complete locally */
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 2ed65b0..1f22a30 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -13,6 +13,7 @@
#include <linux/hrtimer.h>
#include <linux/kref.h>
#include <linux/workqueue.h>
+#include <linux/jump_label.h>
#include <linux/atomic.h>
#include <asm/ptrace.h>
@@ -474,12 +475,13 @@ extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
#ifdef CONFIG_IRQ_FORCED_THREADING
# ifdef CONFIG_PREEMPT_RT
-# define force_irqthreads (true)
+# define force_irqthreads() (true)
# else
-extern bool force_irqthreads;
+DECLARE_STATIC_KEY_FALSE(force_irqthreads_key);
+# define force_irqthreads() (static_branch_unlikely(&force_irqthreads_key))
# endif
#else
-#define force_irqthreads (0)
+#define force_irqthreads() (false)
#endif
#ifndef local_softirq_pending
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 766468a..bb3c51d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -25,12 +25,11 @@
#include "internals.h"
#if defined(CONFIG_IRQ_FORCED_THREADING) && !defined(CONFIG_PREEMPT_RT)
-__read_mostly bool force_irqthreads;
-EXPORT_SYMBOL_GPL(force_irqthreads);
+DEFINE_STATIC_KEY_FALSE(force_irqthreads_key);
static int __init setup_forced_irqthreads(char *arg)
{
- force_irqthreads = true;
+ static_branch_enable(&force_irqthreads_key);
return 0;
}
early_param("threadirqs", setup_forced_irqthreads);
@@ -1260,8 +1259,8 @@ static int irq_thread(void *data)
irqreturn_t (*handler_fn)(struct irq_desc *desc,
struct irqaction *action);
- if (force_irqthreads && test_bit(IRQTF_FORCED_THREAD,
- &action->thread_flags))
+ if (force_irqthreads() && test_bit(IRQTF_FORCED_THREAD,
+ &action->thread_flags))
handler_fn = irq_forced_thread_fn;
else
handler_fn = irq_thread_fn;
diff --git a/kernel/softirq.c b/kernel/softirq.c
index f3a0121..322b65d 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -422,7 +422,7 @@ static inline void invoke_softirq(void)
if (ksoftirqd_running(local_softirq_pending()))
return;
- if (!force_irqthreads || !__this_cpu_read(ksoftirqd)) {
+ if (!force_irqthreads() || !__this_cpu_read(ksoftirqd)) {
#ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
/*
* We can safely execute softirq on the current stack if
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip: irq/core] genirq: Change force_irqthreads to a static key
2021-06-02 18:03 [PATCH] genirq: change force_irqthreads from bool test to static key Tanner Love
2021-06-02 21:05 ` Kees Cook
2021-08-10 14:12 ` [tip: irq/core] genirq: Change force_irqthreads to a " tip-bot2 for Tanner Love
@ 2021-08-10 20:52 ` tip-bot2 for Tanner Love
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Tanner Love @ 2021-08-10 20:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: Eric Dumazet, Tanner Love, Thomas Gleixner, Kees Cook, x86,
linux-kernel, maz
The following commit has been merged into the irq/core branch of tip:
Commit-ID: 91cc470e797828d779cd4c1efbe8519bcb358bae
Gitweb: https://git.kernel.org/tip/91cc470e797828d779cd4c1efbe8519bcb358bae
Author: Tanner Love <tannerlove@google.com>
AuthorDate: Wed, 02 Jun 2021 14:03:38 -04:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Tue, 10 Aug 2021 22:50:07 +02:00
genirq: Change force_irqthreads to a static key
With CONFIG_IRQ_FORCED_THREADING=y, testing the boolean force_irqthreads
could incur a cache line miss in invoke_softirq() and other places.
Replace the test with a static key to avoid the potential cache miss.
[ tglx: Dropped the IDE part, removed the export and updated blk-mq ]
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Tanner Love <tannerlove@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20210602180338.3324213-1-tannerlove.kernel@gmail.com
---
block/blk-mq.c | 2 +-
include/linux/interrupt.h | 8 +++++---
kernel/irq/manage.c | 11 +++++------
kernel/softirq.c | 2 +-
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c4ac51..572d8ab 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -606,7 +606,7 @@ static inline bool blk_mq_complete_need_ipi(struct request *rq)
* This is probably worse than completing the request on a different
* cache domain.
*/
- if (force_irqthreads)
+ if (force_irqthreads())
return false;
/* same CPU or cache domain? Complete locally */
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 2ed65b0..1f22a30 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -13,6 +13,7 @@
#include <linux/hrtimer.h>
#include <linux/kref.h>
#include <linux/workqueue.h>
+#include <linux/jump_label.h>
#include <linux/atomic.h>
#include <asm/ptrace.h>
@@ -474,12 +475,13 @@ extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state which,
#ifdef CONFIG_IRQ_FORCED_THREADING
# ifdef CONFIG_PREEMPT_RT
-# define force_irqthreads (true)
+# define force_irqthreads() (true)
# else
-extern bool force_irqthreads;
+DECLARE_STATIC_KEY_FALSE(force_irqthreads_key);
+# define force_irqthreads() (static_branch_unlikely(&force_irqthreads_key))
# endif
#else
-#define force_irqthreads (0)
+#define force_irqthreads() (false)
#endif
#ifndef local_softirq_pending
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 766468a..34a66c4 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -25,12 +25,11 @@
#include "internals.h"
#if defined(CONFIG_IRQ_FORCED_THREADING) && !defined(CONFIG_PREEMPT_RT)
-__read_mostly bool force_irqthreads;
-EXPORT_SYMBOL_GPL(force_irqthreads);
+DEFINE_STATIC_KEY_FALSE(force_irqthreads_key);
static int __init setup_forced_irqthreads(char *arg)
{
- force_irqthreads = true;
+ static_branch_enable(&force_irqthreads_key);
return 0;
}
early_param("threadirqs", setup_forced_irqthreads);
@@ -1260,8 +1259,8 @@ static int irq_thread(void *data)
irqreturn_t (*handler_fn)(struct irq_desc *desc,
struct irqaction *action);
- if (force_irqthreads && test_bit(IRQTF_FORCED_THREAD,
- &action->thread_flags))
+ if (force_irqthreads() && test_bit(IRQTF_FORCED_THREAD,
+ &action->thread_flags))
handler_fn = irq_forced_thread_fn;
else
handler_fn = irq_thread_fn;
@@ -1322,7 +1321,7 @@ EXPORT_SYMBOL_GPL(irq_wake_thread);
static int irq_setup_forced_threading(struct irqaction *new)
{
- if (!force_irqthreads)
+ if (!force_irqthreads())
return 0;
if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))
return 0;
diff --git a/kernel/softirq.c b/kernel/softirq.c
index f3a0121..322b65d 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -422,7 +422,7 @@ static inline void invoke_softirq(void)
if (ksoftirqd_running(local_softirq_pending()))
return;
- if (!force_irqthreads || !__this_cpu_read(ksoftirqd)) {
+ if (!force_irqthreads() || !__this_cpu_read(ksoftirqd)) {
#ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
/*
* We can safely execute softirq on the current stack if
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-10 20:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-02 18:03 [PATCH] genirq: change force_irqthreads from bool test to static key Tanner Love
2021-06-02 21:05 ` Kees Cook
2021-08-10 14:12 ` [tip: irq/core] genirq: Change force_irqthreads to a " tip-bot2 for Tanner Love
2021-08-10 20:52 ` tip-bot2 for Tanner Love
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.