From: Thomas Gleixner <tglx@linutronix.de>
To: Doug Anderson <dianders@chromium.org>,
Bitao Hu <yaoma@linux.alibaba.com>
Cc: liusong@linux.alibaba.com, akpm@linux-foundation.org,
pmladek@suse.com, kernelfans@gmail.com, deller@gmx.de,
npiggin@gmail.com, tsbogend@alpha.franken.de,
James.Bottomley@hansenpartnership.com, jan.kiszka@siemens.com,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
Date: Fri, 01 Mar 2024 20:22:54 +0100 [thread overview]
Message-ID: <87plwdwycx.ffs@tglx> (raw)
In-Reply-To: <CAD=FV=U1b+8atmju_w4eRmVKmSqjj6WCsy5EawYqj31fQ1kvrw@mail.gmail.com>
Doug!
On Wed, Feb 28 2024 at 14:44, Doug Anderson wrote:
> I won't insist on it, but I continue to worry about memory
> implications with large numbers of CPUs. With a 4-byte int, 8192 max
> CPUs, and 100 IRQs the extra "ref" value takes up over 3MB of memory
> (8192 * 4 bytes * 100).
>
> Technically, you could add a new symbol like "config
> NEED_IRQ_SNAPSHOTS". This wouldn't be a symbol selectable by the end
> user but would automatically be selected by "config
> SOFTLOCKUP_DETECTOR_INTR_STORM". If the config wasn't defined then the
> struct wouldn't contain "ref" and the snapshot routines would just be
> static inline stubs.
>
> Maybe Thomas has an opinion about whether this is something to worry
> about. Worst case it wouldn't be hard to do in a follow-up patch.
I'd say it makes sense to give people a choice to save memory especially
when the softlock detector code is not enabled.
It's rather straight forward to do.
Thanks,
tglx
---
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -24,7 +24,9 @@ struct pt_regs;
*/
struct irqstat {
unsigned int cnt;
+#ifdef CONFIG_GENIRQ_STAT_SNAPSHOT
unsigned int ref;
+#endif
};
/**
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -978,6 +978,7 @@ static unsigned int kstat_irqs(unsigned
return sum;
}
+#ifdef CONFIG_GENIRQ_STAT_SNAPSHOT
void kstat_snapshot_irqs(void)
{
struct irq_desc *desc;
@@ -998,6 +999,7 @@ unsigned int kstat_get_irq_since_snapsho
return 0;
return this_cpu_read(desc->kstat_irqs->cnt) - this_cpu_read(desc->kstat_irqs->ref);
}
+#endif
/**
* kstat_irqs_usr - Get the statistics for an interrupt from thread context
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -108,6 +108,10 @@ config GENERIC_IRQ_MATRIX_ALLOCATOR
config GENERIC_IRQ_RESERVATION_MODE
bool
+# Snapshot for interrupt statistics
+config GENERIC_IRQ_STAT_SNAPSHOT
+ bool
+
# Support forced irq threading
config IRQ_FORCED_THREADING
bool
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Doug Anderson <dianders@chromium.org>,
Bitao Hu <yaoma@linux.alibaba.com>
Cc: pmladek@suse.com, tsbogend@alpha.franken.de,
linux-parisc@vger.kernel.org, jan.kiszka@siemens.com,
deller@gmx.de, liusong@linux.alibaba.com, npiggin@gmail.com,
linux-kernel@vger.kernel.org,
James.Bottomley@hansenpartnership.com, kernelfans@gmail.com,
akpm@linux-foundation.org, linux-mips@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
Date: Fri, 01 Mar 2024 20:22:54 +0100 [thread overview]
Message-ID: <87plwdwycx.ffs@tglx> (raw)
In-Reply-To: <CAD=FV=U1b+8atmju_w4eRmVKmSqjj6WCsy5EawYqj31fQ1kvrw@mail.gmail.com>
Doug!
On Wed, Feb 28 2024 at 14:44, Doug Anderson wrote:
> I won't insist on it, but I continue to worry about memory
> implications with large numbers of CPUs. With a 4-byte int, 8192 max
> CPUs, and 100 IRQs the extra "ref" value takes up over 3MB of memory
> (8192 * 4 bytes * 100).
>
> Technically, you could add a new symbol like "config
> NEED_IRQ_SNAPSHOTS". This wouldn't be a symbol selectable by the end
> user but would automatically be selected by "config
> SOFTLOCKUP_DETECTOR_INTR_STORM". If the config wasn't defined then the
> struct wouldn't contain "ref" and the snapshot routines would just be
> static inline stubs.
>
> Maybe Thomas has an opinion about whether this is something to worry
> about. Worst case it wouldn't be hard to do in a follow-up patch.
I'd say it makes sense to give people a choice to save memory especially
when the softlock detector code is not enabled.
It's rather straight forward to do.
Thanks,
tglx
---
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -24,7 +24,9 @@ struct pt_regs;
*/
struct irqstat {
unsigned int cnt;
+#ifdef CONFIG_GENIRQ_STAT_SNAPSHOT
unsigned int ref;
+#endif
};
/**
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -978,6 +978,7 @@ static unsigned int kstat_irqs(unsigned
return sum;
}
+#ifdef CONFIG_GENIRQ_STAT_SNAPSHOT
void kstat_snapshot_irqs(void)
{
struct irq_desc *desc;
@@ -998,6 +999,7 @@ unsigned int kstat_get_irq_since_snapsho
return 0;
return this_cpu_read(desc->kstat_irqs->cnt) - this_cpu_read(desc->kstat_irqs->ref);
}
+#endif
/**
* kstat_irqs_usr - Get the statistics for an interrupt from thread context
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -108,6 +108,10 @@ config GENERIC_IRQ_MATRIX_ALLOCATOR
config GENERIC_IRQ_RESERVATION_MODE
bool
+# Snapshot for interrupt statistics
+config GENERIC_IRQ_STAT_SNAPSHOT
+ bool
+
# Support forced irq threading
config IRQ_FORCED_THREADING
bool
next prev parent reply other threads:[~2024-03-01 19:22 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-28 7:22 [PATCHv11 0/4] *** Detect interrupt storm in softlockup *** Bitao Hu
2024-02-28 7:22 ` Bitao Hu
2024-02-28 7:22 ` [PATCHv11 1/4] watchdog/softlockup: low-overhead detection of interrupt storm Bitao Hu
2024-02-28 7:22 ` Bitao Hu
2024-02-28 7:22 ` [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics Bitao Hu
2024-02-28 7:22 ` Bitao Hu
2024-02-28 22:44 ` Doug Anderson
2024-02-28 22:44 ` Doug Anderson
2024-03-01 19:22 ` Thomas Gleixner [this message]
2024-03-01 19:22 ` Thomas Gleixner
2024-03-04 12:00 ` Bitao Hu
2024-03-04 12:00 ` Bitao Hu
2024-03-04 14:24 ` Thomas Gleixner
2024-03-04 14:24 ` Thomas Gleixner
2024-03-05 10:57 ` Bitao Hu
2024-03-05 10:57 ` Bitao Hu
2024-03-05 16:57 ` Thomas Gleixner
2024-03-05 16:57 ` Thomas Gleixner
2024-03-06 11:09 ` Bitao Hu
2024-03-06 11:09 ` Bitao Hu
2024-02-28 7:22 ` [PATCHv11 3/4] genirq: Avoid summation loops for /proc/interrupts Bitao Hu
2024-02-28 7:22 ` Bitao Hu
2024-02-28 22:44 ` Doug Anderson
2024-02-28 22:44 ` Doug Anderson
2024-02-28 7:22 ` [PATCHv11 4/4] watchdog/softlockup: report the most frequent interrupts Bitao Hu
2024-02-28 7:22 ` Bitao Hu
2024-02-28 22:44 ` Doug Anderson
2024-02-28 22:44 ` Doug Anderson
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=87plwdwycx.ffs@tglx \
--to=tglx@linutronix.de \
--cc=James.Bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=deller@gmx.de \
--cc=dianders@chromium.org \
--cc=jan.kiszka@siemens.com \
--cc=kernelfans@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=liusong@linux.alibaba.com \
--cc=npiggin@gmail.com \
--cc=pmladek@suse.com \
--cc=tsbogend@alpha.franken.de \
--cc=yaoma@linux.alibaba.com \
/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 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.