From: "Cédric Le Goater" <clg@kaod.org>
To: Thomas Gleixner <tglx@linutronix.de>,
Nicholas Piggin <npiggin@gmail.com>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] genirq: reduce irqdebug bouncing cachelines
Date: Mon, 12 Apr 2021 11:06:17 +0200 [thread overview]
Message-ID: <2dae4501-6f01-1b32-4b69-1dfc94c93425@kaod.org> (raw)
In-Reply-To: <87im4u2vxx.ffs@nanos.tec.linutronix.de>
Hello,
On 4/10/21 1:58 PM, Thomas Gleixner wrote:
> Nicholas,
>
> On Fri, Apr 02 2021 at 23:20, Nicholas Piggin wrote:
>> note_interrupt increments desc->irq_count for each interrupt even for
>> percpu interrupt handlers, even when they are handled successfully. This
>> causes cacheline bouncing and limits scalability.
>>
>> Instead of incrementing irq_count every time, only start incrementing it
>> after seeing an unhandled irq, which should avoid the cache line
>> bouncing in the common path.
>>
>> This actually should give better consistency in handling misbehaving
>> irqs too, because instead of the first unhandled irq arriving at an
>> arbitrary point in the irq_count cycle, its arrival will begin the
>> irq_count cycle.
>
> I've applied that because it makes sense in general, but I think the whole
> call to note_interrupt() can be avoided or return early when interrupts
> would be marked accordingly. For IPI handlers which always return
> HANDLED the whole procedure is pretty pointless to begin with.
>
> Something like the completely untested below.
>
> Thanks,
>
> tglx
> ---
> include/linux/interrupt.h | 3 +++
> include/linux/irq.h | 2 ++
> kernel/irq/manage.c | 2 ++
> kernel/irq/settings.h | 12 ++++++++++++
> kernel/irq/spurious.c | 2 +-
> 5 files changed, 20 insertions(+), 1 deletion(-)
>
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -64,6 +64,8 @@
> * IRQF_NO_AUTOEN - Don't enable IRQ or NMI automatically when users request it.
> * Users will enable it explicitly by enable_irq() or enable_nmi()
> * later.
> + * IRQF_NO_DEBUG - Exclude from runnaway detection for IPI and similar handlers,
> + * depends on IRQF_PERCPU.
> */
> #define IRQF_SHARED 0x00000080
> #define IRQF_PROBE_SHARED 0x00000100
> @@ -78,6 +80,7 @@
> #define IRQF_EARLY_RESUME 0x00020000
> #define IRQF_COND_SUSPEND 0x00040000
> #define IRQF_NO_AUTOEN 0x00080000
> +#define IRQF_NO_DEBUG 0x00100000
>
> #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
>
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -72,6 +72,7 @@ enum irqchip_irq_state;
> * mechanism and from core side polling.
> * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
> * IRQ_HIDDEN - Don't show up in /proc/interrupts
> + * IRQ_NO_DEBUG - Exclude from note_interrupt() debugging
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -99,6 +100,7 @@ enum {
> IRQ_IS_POLLED = (1 << 18),
> IRQ_DISABLE_UNLAZY = (1 << 19),
> IRQ_HIDDEN = (1 << 20),
> + IRQ_NO_DEBUG = (1 << 21),
> };
>
> #define IRQF_MODIFY_MASK \
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1682,6 +1682,8 @@ static int
> if (new->flags & IRQF_PERCPU) {
> irqd_set(&desc->irq_data, IRQD_PER_CPU);
> irq_settings_set_per_cpu(desc);
> + if (new->flags & IRQF_NO_DEBUG)
> + irq_settings_set_no_debug(desc);
> }
>
> if (new->flags & IRQF_ONESHOT)
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -18,6 +18,7 @@ enum {
> _IRQ_IS_POLLED = IRQ_IS_POLLED,
> _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
> _IRQ_HIDDEN = IRQ_HIDDEN,
> + _IRQ_NO_DEBUG = IRQ_NO_DEBUG,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -33,6 +34,7 @@ enum {
> #define IRQ_IS_POLLED GOT_YOU_MORON
> #define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
> #define IRQ_HIDDEN GOT_YOU_MORON
> +#define IRQ_NO_DEBUG GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -174,3 +176,13 @@ static inline bool irq_settings_is_hidde
> {
> return desc->status_use_accessors & _IRQ_HIDDEN;
> }
> +
> +static inline void irq_settings_set_no_debug(struct irq_desc *desc)
> +{
> + desc->status_use_accessors |= _IRQ_NO_DEBUG;
> +}
> +
> +static inline bool irq_settings_no_debug(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors & _IRQ_NO_DEBUG;
> +}
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -274,7 +274,7 @@ void note_interrupt(struct irq_desc *des
> unsigned int irq;
>
> if (desc->istate & IRQS_POLL_INPROGRESS ||
> - irq_settings_is_polled(desc))
> + irq_settings_is_polled(desc) | irq_settings_no_debug(desc))
Shouldn't it be '||' instead ?
> return;
>
> if (bad_action_ret(action_ret)) {
>
We could test irq_settings_no_debug() directly under handle_nested_irq()
and handle_irq_event_percpu() to avoid calling note_interrupt(), just
like we do for noirqdebug.
Thanks,
C.
next prev parent reply other threads:[~2021-04-12 9:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-02 13:20 [PATCH] genirq: reduce irqdebug bouncing cachelines Nicholas Piggin
2021-04-10 11:38 ` [tip: irq/core] genirq: Reduce irqdebug cacheline bouncing tip-bot2 for Nicholas Piggin
2021-04-10 11:58 ` [PATCH] genirq: reduce irqdebug bouncing cachelines Thomas Gleixner
2021-04-12 9:06 ` Cédric Le Goater [this message]
2021-04-12 12:43 ` Thomas Gleixner
2021-04-13 12:16 ` Cédric Le Goater
2021-04-13 20:24 ` Thomas Gleixner
2021-04-14 13:13 ` Cédric Le Goater
2021-05-15 17:01 ` Cédric Le Goater
2021-05-17 18:04 ` [tip: irq/core] genirq: Add a IRQF_NO_DEBUG flag tip-bot2 for Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2dae4501-6f01-1b32-4b69-1dfc94c93425@kaod.org \
--to=clg@kaod.org \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@gmail.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).