[PATCHv10,2/4] genirq: Provide a snapshot mechanism for interrupt statistics
Commit Message
The soft lockup detector lacks a mechanism to identify interrupt storms
as root cause of a lockup. To enable this the detector needs a
mechanism to snapshot the interrupt count statistics on a CPU when the
detector observes a potential lockup scenario and compare that against
the interrupt count when it warns about the lockup later on. The number
of interrupts in that period give a hint whether the lockup might be
caused by an interrupt storm.
Instead of having extra storage in the lockup detector and accessing
the internals of the interrupt descriptor directly, convert the per CPU
irq_desc::kstat_irq member to a data structure which contains the
counter plus a snapshot member and provide interfaces to take a
snapshot of all interrupts on the current CPU and to retrieve the delta
of a specific interrupt later on.
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Bitao Hu <yaoma@linux.alibaba.com>
---
arch/mips/dec/setup.c | 2 +-
arch/parisc/kernel/smp.c | 2 +-
arch/powerpc/kvm/book3s_hv_rm_xics.c | 2 +-
include/linux/irqdesc.h | 9 ++++++--
include/linux/kernel_stat.h | 3 +++
kernel/irq/internals.h | 2 +-
kernel/irq/irqdesc.c | 34 ++++++++++++++++++++++------
kernel/irq/proc.c | 5 ++--
scripts/gdb/linux/interrupts.py | 6 ++---
9 files changed, 46 insertions(+), 19 deletions(-)
Comments
在 2024/2/26 10:09, Bitao Hu 写道:
> The soft lockup detector lacks a mechanism to identify interrupt storms
> as root cause of a lockup. To enable this the detector needs a
> mechanism to snapshot the interrupt count statistics on a CPU when the
> detector observes a potential lockup scenario and compare that against
> the interrupt count when it warns about the lockup later on. The number
> of interrupts in that period give a hint whether the lockup might be
> caused by an interrupt storm.
>
> Instead of having extra storage in the lockup detector and accessing
> the internals of the interrupt descriptor directly, convert the per CPU
> irq_desc::kstat_irq member to a data structure which contains the
> counter plus a snapshot member and provide interfaces to take a
> snapshot of all interrupts on the current CPU and to retrieve the delta
> of a specific interrupt later on.
>
> Originally-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Bitao Hu <yaoma@linux.alibaba.com>
> ---
> arch/mips/dec/setup.c | 2 +-
> arch/parisc/kernel/smp.c | 2 +-
> arch/powerpc/kvm/book3s_hv_rm_xics.c | 2 +-
> include/linux/irqdesc.h | 9 ++++++--
> include/linux/kernel_stat.h | 3 +++
> kernel/irq/internals.h | 2 +-
> kernel/irq/irqdesc.c | 34 ++++++++++++++++++++++------
> kernel/irq/proc.c | 5 ++--
> scripts/gdb/linux/interrupts.py | 6 ++---
> 9 files changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/arch/mips/dec/setup.c b/arch/mips/dec/setup.c
> index 6c3704f51d0d..87f0a1436bf9 100644
> --- a/arch/mips/dec/setup.c
> +++ b/arch/mips/dec/setup.c
> @@ -756,7 +756,7 @@ void __init arch_init_irq(void)
> NULL))
> pr_err("Failed to register fpu interrupt\n");
> desc_fpu = irq_to_desc(irq_fpu);
> - fpu_kstat_irq = this_cpu_ptr(desc_fpu->kstat_irqs);
> + fpu_kstat_irq = this_cpu_ptr(&desc_fpu->kstat_irqs->cnt);
> }
> if (dec_interrupt[DEC_IRQ_CASCADE] >= 0) {
> if (request_irq(dec_interrupt[DEC_IRQ_CASCADE], no_action,
> diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
> index 444154271f23..800eb64e91ad 100644
> --- a/arch/parisc/kernel/smp.c
> +++ b/arch/parisc/kernel/smp.c
> @@ -344,7 +344,7 @@ static int smp_boot_one_cpu(int cpuid, struct task_struct *idle)
> struct irq_desc *desc = irq_to_desc(i);
>
> if (desc && desc->kstat_irqs)
> - *per_cpu_ptr(desc->kstat_irqs, cpuid) = 0;
> + *per_cpu_ptr(desc->kstat_irqs, cpuid) = (struct irqstat) { };
> }
> #endif
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> index e42984878503..f2636414d82a 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
> @@ -837,7 +837,7 @@ static inline void this_cpu_inc_rm(unsigned int __percpu *addr)
> */
> static void kvmppc_rm_handle_irq_desc(struct irq_desc *desc)
> {
> - this_cpu_inc_rm(desc->kstat_irqs);
> + this_cpu_inc_rm(&desc->kstat_irqs->cnt);
> __this_cpu_inc(kstat.irqs_sum);
> }
>
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index d9451d456a73..2912b1998670 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -17,6 +17,11 @@ struct irq_desc;
> struct irq_domain;
> struct pt_regs;
>
> +struct irqstat {
> + unsigned int cnt;
> + unsigned int ref;
> +};
> +
> /**
> * struct irq_desc - interrupt descriptor
> * @irq_common_data: per irq and chip data passed down to chip functions
> @@ -55,7 +60,7 @@ struct pt_regs;
> struct irq_desc {
> struct irq_common_data irq_common_data;
> struct irq_data irq_data;
> - unsigned int __percpu *kstat_irqs;
> + struct irqstat __percpu *kstat_irqs;
> irq_flow_handler_t handle_irq;
> struct irqaction *action; /* IRQ action list */
> unsigned int status_use_accessors;
> @@ -119,7 +124,7 @@ extern struct irq_desc irq_desc[NR_IRQS];
> static inline unsigned int irq_desc_kstat_cpu(struct irq_desc *desc,
> unsigned int cpu)
> {
> - return desc->kstat_irqs ? *per_cpu_ptr(desc->kstat_irqs, cpu) : 0;
> + return desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, cpu) : 0;
> }
>
> static inline struct irq_desc *irq_data_to_desc(struct irq_data *data)
> diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h
> index 9935f7ecbfb9..98b3043ea5e6 100644
> --- a/include/linux/kernel_stat.h
> +++ b/include/linux/kernel_stat.h
> @@ -79,6 +79,9 @@ static inline unsigned int kstat_cpu_softirqs_sum(int cpu)
> return sum;
> }
>
> +extern void kstat_snapshot_irqs(void);
> +extern unsigned int kstat_get_irq_since_snapshot(unsigned int irq);
> +
> /*
> * Number of interrupts per specific IRQ source, since bootup
> */
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index bcc7f21db9ee..1d92532c2aae 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -258,7 +258,7 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
>
> static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)
> {
> - __this_cpu_inc(*desc->kstat_irqs);
> + __this_cpu_inc(desc->kstat_irqs->cnt);
> __this_cpu_inc(kstat.irqs_sum);
> }
>
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 27ca1c866f29..9cd17080b2d8 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -122,7 +122,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
> desc->name = NULL;
> desc->owner = owner;
> for_each_possible_cpu(cpu)
> - *per_cpu_ptr(desc->kstat_irqs, cpu) = 0;
> + *per_cpu_ptr(desc->kstat_irqs, cpu) = (struct irqstat) { };
> desc_smp_init(desc, node, affinity);
> }
>
> @@ -418,8 +418,8 @@ static struct irq_desc *alloc_desc(int irq, int node, unsigned int flags,
> desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);
> if (!desc)
> return NULL;
> - /* allocate based on nr_cpu_ids */
> - desc->kstat_irqs = alloc_percpu(unsigned int);
> +
> + desc->kstat_irqs = alloc_percpu(struct irqstat);
> if (!desc->kstat_irqs)
> goto err_desc;
>
> @@ -593,7 +593,7 @@ int __init early_irq_init(void)
> count = ARRAY_SIZE(irq_desc);
>
> for (i = 0; i < count; i++) {
> - desc[i].kstat_irqs = alloc_percpu(unsigned int);
> + desc[i].kstat_irqs = alloc_percpu(struct irqstat);
> alloc_masks(&desc[i], node);
> raw_spin_lock_init(&desc[i].lock);
> lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
> @@ -952,8 +952,7 @@ unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - return desc && desc->kstat_irqs ?
> - *per_cpu_ptr(desc->kstat_irqs, cpu) : 0;
> + return desc && desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, cpu) : 0;
> }
>
> static bool irq_is_nmi(struct irq_desc *desc)
> @@ -975,10 +974,31 @@ static unsigned int kstat_irqs(unsigned int irq)
> return data_race(desc->tot_count);
>
> for_each_possible_cpu(cpu)
> - sum += data_race(*per_cpu_ptr(desc->kstat_irqs, cpu));
> + sum += data_race(per_cpu(desc->kstat_irqs->cnt, cpu));
> return sum;
> }
>
> +void kstat_snapshot_irqs(void)
> +{
> + struct irq_desc *desc;
> + unsigned int irq;
> +
> + for_each_irq_desc(irq, desc) {
> + if (!desc->kstat_irqs)
> + continue;
> + this_cpu_write(desc->kstat_irqs->ref, this_cpu_read(desc->kstat_irqs->cnt));
> + }
> +}
> +
> +unsigned int kstat_get_irq_since_snapshot(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc || !desc->kstat_irqs)
> + return 0;
> + return this_cpu_read(desc->kstat_irqs->cnt) - this_cpu_read(desc->kstat_irqs->ref);
> +}
> +
> /**
> * kstat_irqs_usr - Get the statistics for an interrupt from thread context
> * @irq: The interrupt number
> diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
> index 623b8136e9af..6954e0a02047 100644
> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -490,7 +490,7 @@ int show_interrupts(struct seq_file *p, void *v)
>
> if (desc->kstat_irqs) {
> for_each_online_cpu(j)
> - any_count |= data_race(*per_cpu_ptr(desc->kstat_irqs, j));
> + any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j));
> }
>
> if ((!desc->action || irq_desc_is_chained(desc)) && !any_count)
> @@ -498,8 +498,7 @@ int show_interrupts(struct seq_file *p, void *v)
>
> seq_printf(p, "%*d: ", prec, i);
> for_each_online_cpu(j)
> - seq_printf(p, "%10u ", desc->kstat_irqs ?
> - *per_cpu_ptr(desc->kstat_irqs, j) : 0);
> + seq_printf(p, "%10u ", desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, j) : 0);
>
> raw_spin_lock_irqsave(&desc->lock, flags);
> if (desc->irq_data.chip) {
> diff --git a/scripts/gdb/linux/interrupts.py b/scripts/gdb/linux/interrupts.py
> index ef478e273791..7e50f3b9dfad 100644
> --- a/scripts/gdb/linux/interrupts.py
> +++ b/scripts/gdb/linux/interrupts.py
> @@ -37,7 +37,7 @@ def show_irq_desc(prec, irq):
> any_count = 0
> if desc['kstat_irqs']:
> for cpu in cpus.each_online_cpu():
> - any_count += cpus.per_cpu(desc['kstat_irqs'], cpu)
> + any_count += cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt']
>
> if (desc['action'] == 0 or irq_desc_is_chained(desc)) and any_count == 0:
> return text;
> @@ -45,7 +45,7 @@ def show_irq_desc(prec, irq):
> text += "%*d: " % (prec, irq)
> for cpu in cpus.each_online_cpu():
> if desc['kstat_irqs']:
> - count = cpus.per_cpu(desc['kstat_irqs'], cpu)
> + count = cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt']
> else:
> count = 0
> text += "%10u" % (count)
> @@ -177,7 +177,7 @@ def arm_common_show_interrupts(prec):
> if desc == 0:
> continue
> for cpu in cpus.each_online_cpu():
> - text += "%10u" % (cpus.per_cpu(desc['kstat_irqs'], cpu))
> + text += "%10u" % (cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt'])
> text += " %s" % (ipi_types[ipi].string())
> text += "\n"
> return text
Looks good.
For the newly added struct irqstat, adding annotated comments to explain
each field would be beneficial.
Reviewed-by: Liu Song <liusong@linux.alibaba.com>
@@ -756,7 +756,7 @@ void __init arch_init_irq(void)
NULL))
pr_err("Failed to register fpu interrupt\n");
desc_fpu = irq_to_desc(irq_fpu);
- fpu_kstat_irq = this_cpu_ptr(desc_fpu->kstat_irqs);
+ fpu_kstat_irq = this_cpu_ptr(&desc_fpu->kstat_irqs->cnt);
}
if (dec_interrupt[DEC_IRQ_CASCADE] >= 0) {
if (request_irq(dec_interrupt[DEC_IRQ_CASCADE], no_action,
@@ -344,7 +344,7 @@ static int smp_boot_one_cpu(int cpuid, struct task_struct *idle)
struct irq_desc *desc = irq_to_desc(i);
if (desc && desc->kstat_irqs)
- *per_cpu_ptr(desc->kstat_irqs, cpuid) = 0;
+ *per_cpu_ptr(desc->kstat_irqs, cpuid) = (struct irqstat) { };
}
#endif
@@ -837,7 +837,7 @@ static inline void this_cpu_inc_rm(unsigned int __percpu *addr)
*/
static void kvmppc_rm_handle_irq_desc(struct irq_desc *desc)
{
- this_cpu_inc_rm(desc->kstat_irqs);
+ this_cpu_inc_rm(&desc->kstat_irqs->cnt);
__this_cpu_inc(kstat.irqs_sum);
}
@@ -17,6 +17,11 @@ struct irq_desc;
struct irq_domain;
struct pt_regs;
+struct irqstat {
+ unsigned int cnt;
+ unsigned int ref;
+};
+
/**
* struct irq_desc - interrupt descriptor
* @irq_common_data: per irq and chip data passed down to chip functions
@@ -55,7 +60,7 @@ struct pt_regs;
struct irq_desc {
struct irq_common_data irq_common_data;
struct irq_data irq_data;
- unsigned int __percpu *kstat_irqs;
+ struct irqstat __percpu *kstat_irqs;
irq_flow_handler_t handle_irq;
struct irqaction *action; /* IRQ action list */
unsigned int status_use_accessors;
@@ -119,7 +124,7 @@ extern struct irq_desc irq_desc[NR_IRQS];
static inline unsigned int irq_desc_kstat_cpu(struct irq_desc *desc,
unsigned int cpu)
{
- return desc->kstat_irqs ? *per_cpu_ptr(desc->kstat_irqs, cpu) : 0;
+ return desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, cpu) : 0;
}
static inline struct irq_desc *irq_data_to_desc(struct irq_data *data)
@@ -79,6 +79,9 @@ static inline unsigned int kstat_cpu_softirqs_sum(int cpu)
return sum;
}
+extern void kstat_snapshot_irqs(void);
+extern unsigned int kstat_get_irq_since_snapshot(unsigned int irq);
+
/*
* Number of interrupts per specific IRQ source, since bootup
*/
@@ -258,7 +258,7 @@ static inline void irq_state_set_masked(struct irq_desc *desc)
static inline void __kstat_incr_irqs_this_cpu(struct irq_desc *desc)
{
- __this_cpu_inc(*desc->kstat_irqs);
+ __this_cpu_inc(desc->kstat_irqs->cnt);
__this_cpu_inc(kstat.irqs_sum);
}
@@ -122,7 +122,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
desc->name = NULL;
desc->owner = owner;
for_each_possible_cpu(cpu)
- *per_cpu_ptr(desc->kstat_irqs, cpu) = 0;
+ *per_cpu_ptr(desc->kstat_irqs, cpu) = (struct irqstat) { };
desc_smp_init(desc, node, affinity);
}
@@ -418,8 +418,8 @@ static struct irq_desc *alloc_desc(int irq, int node, unsigned int flags,
desc = kzalloc_node(sizeof(*desc), GFP_KERNEL, node);
if (!desc)
return NULL;
- /* allocate based on nr_cpu_ids */
- desc->kstat_irqs = alloc_percpu(unsigned int);
+
+ desc->kstat_irqs = alloc_percpu(struct irqstat);
if (!desc->kstat_irqs)
goto err_desc;
@@ -593,7 +593,7 @@ int __init early_irq_init(void)
count = ARRAY_SIZE(irq_desc);
for (i = 0; i < count; i++) {
- desc[i].kstat_irqs = alloc_percpu(unsigned int);
+ desc[i].kstat_irqs = alloc_percpu(struct irqstat);
alloc_masks(&desc[i], node);
raw_spin_lock_init(&desc[i].lock);
lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
@@ -952,8 +952,7 @@ unsigned int kstat_irqs_cpu(unsigned int irq, int cpu)
{
struct irq_desc *desc = irq_to_desc(irq);
- return desc && desc->kstat_irqs ?
- *per_cpu_ptr(desc->kstat_irqs, cpu) : 0;
+ return desc && desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, cpu) : 0;
}
static bool irq_is_nmi(struct irq_desc *desc)
@@ -975,10 +974,31 @@ static unsigned int kstat_irqs(unsigned int irq)
return data_race(desc->tot_count);
for_each_possible_cpu(cpu)
- sum += data_race(*per_cpu_ptr(desc->kstat_irqs, cpu));
+ sum += data_race(per_cpu(desc->kstat_irqs->cnt, cpu));
return sum;
}
+void kstat_snapshot_irqs(void)
+{
+ struct irq_desc *desc;
+ unsigned int irq;
+
+ for_each_irq_desc(irq, desc) {
+ if (!desc->kstat_irqs)
+ continue;
+ this_cpu_write(desc->kstat_irqs->ref, this_cpu_read(desc->kstat_irqs->cnt));
+ }
+}
+
+unsigned int kstat_get_irq_since_snapshot(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc || !desc->kstat_irqs)
+ return 0;
+ return this_cpu_read(desc->kstat_irqs->cnt) - this_cpu_read(desc->kstat_irqs->ref);
+}
+
/**
* kstat_irqs_usr - Get the statistics for an interrupt from thread context
* @irq: The interrupt number
@@ -490,7 +490,7 @@ int show_interrupts(struct seq_file *p, void *v)
if (desc->kstat_irqs) {
for_each_online_cpu(j)
- any_count |= data_race(*per_cpu_ptr(desc->kstat_irqs, j));
+ any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j));
}
if ((!desc->action || irq_desc_is_chained(desc)) && !any_count)
@@ -498,8 +498,7 @@ int show_interrupts(struct seq_file *p, void *v)
seq_printf(p, "%*d: ", prec, i);
for_each_online_cpu(j)
- seq_printf(p, "%10u ", desc->kstat_irqs ?
- *per_cpu_ptr(desc->kstat_irqs, j) : 0);
+ seq_printf(p, "%10u ", desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, j) : 0);
raw_spin_lock_irqsave(&desc->lock, flags);
if (desc->irq_data.chip) {
@@ -37,7 +37,7 @@ def show_irq_desc(prec, irq):
any_count = 0
if desc['kstat_irqs']:
for cpu in cpus.each_online_cpu():
- any_count += cpus.per_cpu(desc['kstat_irqs'], cpu)
+ any_count += cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt']
if (desc['action'] == 0 or irq_desc_is_chained(desc)) and any_count == 0:
return text;
@@ -45,7 +45,7 @@ def show_irq_desc(prec, irq):
text += "%*d: " % (prec, irq)
for cpu in cpus.each_online_cpu():
if desc['kstat_irqs']:
- count = cpus.per_cpu(desc['kstat_irqs'], cpu)
+ count = cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt']
else:
count = 0
text += "%10u" % (count)
@@ -177,7 +177,7 @@ def arm_common_show_interrupts(prec):
if desc == 0:
continue
for cpu in cpus.each_online_cpu():
- text += "%10u" % (cpus.per_cpu(desc['kstat_irqs'], cpu))
+ text += "%10u" % (cpus.per_cpu(desc['kstat_irqs'], cpu)['cnt'])
text += " %s" % (ipi_types[ipi].string())
text += "\n"
return text