Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2024/3/2 03:22, Thomas Gleixner wrote:
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

I think we should follow Douglas's suggestion by making
"config GENERIC_IRQ_STAT_SNAPSHOT" automatically selectable by
"config SOFTLOCKUP_DETECTOR_INTR_STORM". This can prevent users
from inadvertently disabling "config GENERIC_IRQ_STAT_SNAPSHOT"
while enabling "config SOFTLOCKUP_DETECTOR_INTR_STORM".

Best Regards,
	Bitao Hu

diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 2531f3496ab6..9cf3b2d4c2a8 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -108,6 +108,15 @@ config GENERIC_IRQ_MATRIX_ALLOCATOR
 config GENERIC_IRQ_RESERVATION_MODE
        bool

+# Snapshot for interrupt statistics
+config GENERIC_IRQ_STAT_SNAPSHOT
+       bool
+       help
+
+         Say Y here to enable the kernel to provide a snapshot mechanism
+         for interrupt statistics.
+
+
 # Support forced irq threading
 config IRQ_FORCED_THREADING
        bool
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 49f652674bd8..899b69fcb598 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1032,6 +1032,7 @@ config SOFTLOCKUP_DETECTOR
 config SOFTLOCKUP_DETECTOR_INTR_STORM
        bool "Detect Interrupt Storm in Soft Lockups"
        depends on SOFTLOCKUP_DETECTOR && IRQ_TIME_ACCOUNTING
+       select GENERIC_IRQ_STAT_SNAPSHOT
        default y if NR_CPUS <= 128
        help
          Say Y here to enable the kernel to detect interrupt storm




[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux