[PATCHv13 5/5] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 638.870231] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [swapper/9:0] [ 638.870825] CPU#9 Utilization every 4s during lockup: [ 638.871194] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.871652] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872107] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872563] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873018] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873494] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 638.873994] #1: 330945 irq#7 [ 638.874236] #2: 31 irq#82 [ 638.874493] #3: 10 irq#10 [ 638.874744] #4: 2 irq#89 [ 638.874992] #5: 1 irq#102 ... [ 638.875313] Call trace: [ 638.875315] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu Reviewed-by: Liu Song Reviewed-by: Douglas Anderson --- kernel/watchdog.c | 116 -- 1 file changed, 112 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index ef8ebd31fdab..d12ff74889ed 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -12,22 +12,25 @@ #define pr_fmt(fmt) "watchdog: " fmt -#include #include -#include #include +#include +#include #include +#include #include +#include #include +#include +#include #include #include + #include #include #include -#include #include -#include static DEFINE_MUTEX(watchdog_mutex); @@ -418,13 +421,105 @@ static void print_cpustat(void) } } +#define HARDIRQ_PERCENT_THRESH 50 +#define NUM_HARDIRQ_REPORT 5 +struct irq_counts { + int irq; + u32 counts; +}; + +static DEFINE_PER_CPU(bool, snapshot_taken); + +/* Tabulate the most frequent interrupts. */ +static void tabulate_irq_count(struct irq_counts *irq_counts, int irq, u32 counts, int rank) +{ + int i; + struct irq_counts new_count = {irq, counts}; + + for (i = 0; i < rank; i++) { + if (counts > irq_counts[i].counts) + swap(new_count, irq_counts[i]); + } +} + +/* + * If the hardirq time exceeds HARDIRQ_PERCENT_THRESH% of the sample_period, + * then the cause of softlockup might be interrupt storm. In this case, it + * would be useful to start interrupt counting. + */ +static bool need_counting_irqs(void) +{ + u8 util; + int tail = __this_cpu_read(cpustat_tail); + + tail = (tail + NUM_HARDIRQ_REPORT - 1) % NUM_HARDIRQ_REPORT; + util = __this_cpu_read(cpustat_util[tail][STATS_HARDIRQ]); + return util > HARDIRQ_PERCENT_THRESH; +} + +static void start_counting_irqs(void) +{ + if (!__this_cpu_read(snapshot_taken)) { + kstat_snapshot_irqs(); + __this_cpu_write(snapshot_taken, true); + } +} + +static void stop_counting_irqs(void) +{ + __this_cpu_write(snapshot_taken, false); +} + +static void print_irq_counts(void) +{ + unsigned int i, count; + struct irq_counts irq_counts_sorted[NUM_HARDIRQ_REPORT] = { + {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0} + }; + + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* Outputting the "watchdog" prefix on every line is redundant and not +* concise, and the original alarm information is sufficient for +* positioning in logs, hence here printk() is used instead of pr_crit(). +*/ + printk(KERN_CRIT "CPU#%d Detect HardIRQ Time exceeds %d%%. Most frequent HardIRQs:\n", + smp_processor_id(), HARDIRQ_PERCENT_THRESH); + + for (i = 0; i < NUM_HARDIRQ_REPORT; i++) { + if (irq_counts_sorted[i].irq == -1) + break; + + printk(KERN_CRIT "\t#%u: %-10u\tirq#%d\n", + i + 1, irq_counts_sorted[i].counts, + irq_counts_sorted[i].irq); + } + + /* +* If the hardirq time is less than HARDIRQ_PERCENT_THRESH% in the last +* sample_period, then we suspect the interrupt storm might b
[PATCHv13 4/5] watchdog/softlockup: low-overhead detection of interrupt storm
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore, I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 99 ++- lib/Kconfig.debug | 14 +++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index d7b2125503af..ef8ebd31fdab 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,96 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* Outputting the "watchdog" prefix on every line is redundant and not +* concise, and the original alarm information is sufficient for +* positioning in logs, hence here printk() is used instead of pr_crit().
[PATCHv13 3/5] genirq: Avoid summation loops for /proc/interrupts
show_interrupts() unconditionally accumulates the per CPU interrupt statistics to determine whether an interrupt was ever raised. This can be avoided for all interrupts which are not strictly per CPU and not of type NMI because those interrupts provide already an accumulated counter. The required logic is already implemented in kstat_irqs(). Split the inner access logic out of kstat_irqs() and use it for kstat_irqs() and show_interrupts() to avoid the accumulation loop when possible. Originally-by: Thomas Gleixner Signed-off-by: Bitao Hu Reviewed-by: Liu Song Reviewed-by: Douglas Anderson --- kernel/irq/internals.h | 2 ++ kernel/irq/irqdesc.c | 16 +++- kernel/irq/proc.c | 6 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index 1d92532c2aae..6c43ef3e7308 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -98,6 +98,8 @@ extern void mask_irq(struct irq_desc *desc); extern void unmask_irq(struct irq_desc *desc); extern void unmask_threaded_irq(struct irq_desc *desc); +extern unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask); + #ifdef CONFIG_SPARSE_IRQ static inline void irq_mark_irq(unsigned int irq) { } #else diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index f348faffa7b4..382093196210 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -976,24 +976,30 @@ static bool irq_is_nmi(struct irq_desc *desc) return desc->istate & IRQS_NMI; } -static unsigned int kstat_irqs(unsigned int irq) +unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask) { - struct irq_desc *desc = irq_to_desc(irq); unsigned int sum = 0; int cpu; - if (!desc || !desc->kstat_irqs) - return 0; if (!irq_settings_is_per_cpu_devid(desc) && !irq_settings_is_per_cpu(desc) && !irq_is_nmi(desc)) return data_race(desc->tot_count); - for_each_possible_cpu(cpu) + for_each_cpu(cpu, cpumask) sum += data_race(per_cpu(desc->kstat_irqs->cnt, cpu)); return sum; } +static unsigned int kstat_irqs(unsigned int irq) +{ + struct irq_desc *desc = irq_to_desc(irq); + + if (!desc || !desc->kstat_irqs) + return 0; + return kstat_irqs_desc(desc, cpu_possible_mask); +} + #ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT void kstat_snapshot_irqs(void) diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 6954e0a02047..5c320c3f10a7 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -488,10 +488,8 @@ int show_interrupts(struct seq_file *p, void *v) if (!desc || irq_settings_is_hidden(desc)) goto outsparse; - if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); - } + if (desc->kstat_irqs) + any_count = kstat_irqs_desc(desc, cpu_online_mask); if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) goto outsparse; -- 2.37.1 (Apple Git-137.1)
[PATCHv13 2/5] genirq: Provide a snapshot mechanism for interrupt statistics
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, add a snapshot member to the per CPU irq_desc::kstat_irq structure 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 Signed-off-by: Bitao Hu --- include/linux/irqdesc.h | 4 include/linux/kernel_stat.h | 8 kernel/irq/Kconfig | 4 kernel/irq/irqdesc.c| 25 + 4 files changed, 41 insertions(+) diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index c28612674acb..fd091c35d572 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -20,9 +20,13 @@ struct pt_regs; /** * struct irqstat - interrupt statistics * @cnt: real-time interrupt count + * @ref: snapshot of interrupt count */ struct irqstat { unsigned intcnt; +#ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT + unsigned intref; +#endif }; /** diff --git a/include/linux/kernel_stat.h b/include/linux/kernel_stat.h index 9935f7ecbfb9..9c042c6384bb 100644 --- a/include/linux/kernel_stat.h +++ b/include/linux/kernel_stat.h @@ -79,6 +79,14 @@ static inline unsigned int kstat_cpu_softirqs_sum(int cpu) return sum; } +#ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT +extern void kstat_snapshot_irqs(void); +extern unsigned int kstat_get_irq_since_snapshot(unsigned int irq); +#else +static inline void kstat_snapshot_irqs(void) { } +static inline unsigned int kstat_get_irq_since_snapshot(unsigned int irq) { return 0; } +#endif + /* * Number of interrupts per specific IRQ source, since bootup */ diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index 2531f3496ab6..529adb1f5859 100644 --- 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 diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index b59b79200ad7..f348faffa7b4 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -994,6 +994,31 @@ static unsigned int kstat_irqs(unsigned int irq) return sum; } +#ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT + +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); +} + +#endif + /** * kstat_irqs_usr - Get the statistics for an interrupt from thread context * @irq: The interrupt number -- 2.37.1 (Apple Git-137.1)
[PATCHv13 0/5] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v12 to v13: - Update patch #1 based on the latest kernel code. - From Thomas, split patch #1 into two. The new patch #1 converts kstat_irqs into a struct with just the count in it; the new patch #2 introduces a snapshot mechanism for interrupt statistics. Due to the code being split, I removed the Reviewed-by tags from LiuSong and Douglas in patch #1 and patch #2. Please review it again, and all comments are welcome. - Revised the comment for using printk() instead of pr_crit() to make the reasoning clearer. Changes from v11 to v12: - From Douglas and Thomas, add a new kconfig knob save memory when the softlock detector code is not enabled. - Adjust the order of the patches; patch #1 and patch #2 are related to genirq, while patch #3 and patch #4 are related to watchdog/softlockup, making the dependency relationships clearer. - Add the 'Reviewed-by' tag of Douglas. Changes from v10 to v11: - Only patch #2 and patch #3 have been changed. - Add comments to explain each field of 'struct irqstat' in patch #2. - Split the inner summation logic out of kstat_irqs() and encapsulate it into kstat_irqs_desc() in patch #3. - Adopt Thomas's change log for patch #3. - Add the 'Reviewed-by' tag of Liu Song. Changes from v9 to v10: - The two patches related to 'watchdog/softlockup' remain unchanged. - The majority of the work related to 'genirq' is contributed by Thomas, indicated by adding 'Originally-by' tag. And I'd like to express my gratitude for Thomas's contributions and guidance here. - Adopt Thomas's change log for the snapshot mechanism for interrupt statistics. - Split unrelated change in patch #2 into a separate patch #3. Changes from v8 to v9: - Patch #1 remains unchanged. - From Thomas Gleixner, split patch #2 into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (5): genirq: Convert kstat_irqs to a struct genirq: Provide a snapshot mechanism for interrupt statistics genirq: Avoid summation loops for /proc/interrupts watchdog/softlockup: low-overhead detection of interrupt storm watchdog/softlockup: report the most freq
[PATCHv13 1/5] genirq: Convert kstat_irqs to a struct
The irq_desc::kstat_irqs member is a per-CPU variable of type int, and it is only capable of counting. The snapshot mechanism for interrupt statistics will be added soon, which requires an additional variable to store snapshot. To facilitate expansion, convert kstat_irqs here to a struct containing only the count. Originally-by: Thomas Gleixner Signed-off-by: Bitao Hu --- 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 | 12 ++-- kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 9 - kernel/irq/proc.c| 5 ++--- scripts/gdb/linux/interrupts.py | 6 +++--- 8 files changed, 23 insertions(+), 17 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..c28612674acb 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -17,6 +17,14 @@ struct irq_desc; struct irq_domain; struct pt_regs; +/** + * struct irqstat - interrupt statistics + * @cnt: real-time interrupt count + */ +struct irqstat { + unsigned intcnt; +}; + /** * struct irq_desc - interrupt descriptor * @irq_common_data: per irq and chip data passed down to chip functions @@ -55,7 +63,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 intstatus_use_accessors; @@ -119,7 +127,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/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 4c6b32318ce3..b59b79200ad7 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -134,7 +134,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); } @@ -186,7 +186,7 @@ static int init_desc(struct irq_desc *desc, int irq, int n
Re: [PATCHv12 1/4] genirq: Provide a snapshot mechanism for interrupt statistics
Hi, On 2024/4/9 17:58, Thomas Gleixner wrote: This does not apply anymore. OK, I will update this patch based on the latest kernel code. Also can you please split this apart to convert kstat_irqs to a struct with just the count in it and then add the snapshot mechanics on top. OK, I will split this patch into two. The changelog for the first patch will be as follows. genirq: Convert kstat_irqs to a struct The irq_desc::kstat_irqs member is a per-CPU variable of type int, and it is only capable of counting. The snapshot mechanism for interrupt statistics will be added soon, which requires an additional variable to store snapshot. To facilitate expansion, convert kstat_irqs here to a struct containing only the count. By the way, what do you think of my reason for using printk() instead of pr_crit()? Should I change this part of the code in v13? Besides, are there any other issues with this set of patches? I hope we can resolve all points of contention in v13. Best Regards, Bitao Hu
Re: [PATCHv12 4/4] watchdog/softlockup: report the most frequent interrupts
Hi, Thomas On 2024/3/24 04:43, Thomas Gleixner wrote: On Wed, Mar 06 2024 at 20:52, Bitao Hu wrote: + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ You are not providing any justification why the prefix is not wanted. Just saying 'We do not want' does not cut it and who is 'We'. I certainly not. I really disagree because the prefixes are very useful for searching log files. So not having it makes it harder to filter out for no reason. Regarding the use of printk() instead of pr_crit(), I have had a discussion with Liu Song and Douglas in PATCHv1: https://lore.kernel.org/all/CAD=FV=WEEQeKX=ec3gr-8cks2k0mawn3v0-0yosuret0qcb...@mail.gmail.com/ Please allow me to elaborate on my reasoning. The purpose of the report_cpu_status() function I implemented is similar to that of print_modules(), show_regs(), and dump_stack(). These functions are designed to assist in analyzing the causes of a soft lockup, rather than to report that a soft lockup has occurred. Therefore, I think that adding the "watchdog: " prefix to every line is redundant and not concise. Besides, the existing pr_emerg() in the watchdog.c file is already sufficient for searching useful information in the logs. The information I added, along with the call tree and other data, is located near the line with the "watchdog: " prefix. Are the two reasons I've provided reasonable? Best Regards, Bitao Hu
[PATCHv12 4/4] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 638.870231] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [swapper/9:0] [ 638.870825] CPU#9 Utilization every 4s during lockup: [ 638.871194] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.871652] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872107] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872563] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873018] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873494] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 638.873994] #1: 330945 irq#7 [ 638.874236] #2: 31 irq#82 [ 638.874493] #3: 10 irq#10 [ 638.874744] #4: 2 irq#89 [ 638.874992] #5: 1 irq#102 ... [ 638.875313] Call trace: [ 638.875315] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu Reviewed-by: Liu Song Reviewed-by: Douglas Anderson --- kernel/watchdog.c | 115 -- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 69e72d7e461d..c9d49ae8d045 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -12,22 +12,25 @@ #define pr_fmt(fmt) "watchdog: " fmt -#include #include -#include #include +#include +#include #include +#include #include +#include #include +#include +#include #include #include + #include #include #include -#include #include -#include static DEFINE_MUTEX(watchdog_mutex); @@ -417,13 +420,104 @@ static void print_cpustat(void) } } +#define HARDIRQ_PERCENT_THRESH 50 +#define NUM_HARDIRQ_REPORT 5 +struct irq_counts { + int irq; + u32 counts; +}; + +static DEFINE_PER_CPU(bool, snapshot_taken); + +/* Tabulate the most frequent interrupts. */ +static void tabulate_irq_count(struct irq_counts *irq_counts, int irq, u32 counts, int rank) +{ + int i; + struct irq_counts new_count = {irq, counts}; + + for (i = 0; i < rank; i++) { + if (counts > irq_counts[i].counts) + swap(new_count, irq_counts[i]); + } +} + +/* + * If the hardirq time exceeds HARDIRQ_PERCENT_THRESH% of the sample_period, + * then the cause of softlockup might be interrupt storm. In this case, it + * would be useful to start interrupt counting. + */ +static bool need_counting_irqs(void) +{ + u8 util; + int tail = __this_cpu_read(cpustat_tail); + + tail = (tail + NUM_HARDIRQ_REPORT - 1) % NUM_HARDIRQ_REPORT; + util = __this_cpu_read(cpustat_util[tail][STATS_HARDIRQ]); + return util > HARDIRQ_PERCENT_THRESH; +} + +static void start_counting_irqs(void) +{ + if (!__this_cpu_read(snapshot_taken)) { + kstat_snapshot_irqs(); + __this_cpu_write(snapshot_taken, true); + } +} + +static void stop_counting_irqs(void) +{ + __this_cpu_write(snapshot_taken, false); +} + +static void print_irq_counts(void) +{ + unsigned int i, count; + struct irq_counts irq_counts_sorted[NUM_HARDIRQ_REPORT] = { + {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0} + }; + + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Detect HardIRQ Time exceeds %d%%. Most frequent HardIRQs:\n", + smp_processor_id(), HARDIRQ_PERCENT_THRESH); + + for (i = 0; i < NUM_HARDIRQ_REPORT; i++) { + if (irq_counts_sorted[i].irq == -1) + break; + + printk(KERN_CRIT "\t#%u: %-10u\tirq#%d\n", + i + 1, irq_counts_sorted[i].counts, + irq_counts_sorted[i].irq); + } + + /* +* If the hardirq time is less than HARDIRQ_PERCENT_THRESH% in the last +* sample_period, then we suspect the interrupt storm might be subsiding. +*/ + if (!need_counting_irqs()) +
[PATCHv12 3/4] watchdog/softlockup: low-overhead detection of interrupt storm
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore, I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 98 ++- lib/Kconfig.debug | 14 +++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 81a8862295d6..69e72d7e461d 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,95 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Utilization every %llus during lockup:\n&qu
[PATCHv12 2/4] genirq: Avoid summation loops for /proc/interrupts
show_interrupts() unconditionally accumulates the per CPU interrupt statistics to determine whether an interrupt was ever raised. This can be avoided for all interrupts which are not strictly per CPU and not of type NMI because those interrupts provide already an accumulated counter. The required logic is already implemented in kstat_irqs(). Split the inner access logic out of kstat_irqs() and use it for kstat_irqs() and show_interrupts() to avoid the accumulation loop when possible. Originally-by: Thomas Gleixner Signed-off-by: Bitao Hu Reviewed-by: Liu Song Reviewed-by: Douglas Anderson --- kernel/irq/internals.h | 2 ++ kernel/irq/irqdesc.c | 16 +++- kernel/irq/proc.c | 6 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index 1d92532c2aae..6c43ef3e7308 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -98,6 +98,8 @@ extern void mask_irq(struct irq_desc *desc); extern void unmask_irq(struct irq_desc *desc); extern void unmask_threaded_irq(struct irq_desc *desc); +extern unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask); + #ifdef CONFIG_SPARSE_IRQ static inline void irq_mark_irq(unsigned int irq) { } #else diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 44e187763384..05498788ead5 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -960,24 +960,30 @@ static bool irq_is_nmi(struct irq_desc *desc) return desc->istate & IRQS_NMI; } -static unsigned int kstat_irqs(unsigned int irq) +unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask) { - struct irq_desc *desc = irq_to_desc(irq); unsigned int sum = 0; int cpu; - if (!desc || !desc->kstat_irqs) - return 0; if (!irq_settings_is_per_cpu_devid(desc) && !irq_settings_is_per_cpu(desc) && !irq_is_nmi(desc)) return data_race(desc->tot_count); - for_each_possible_cpu(cpu) + for_each_cpu(cpu, cpumask) sum += data_race(per_cpu(desc->kstat_irqs->cnt, cpu)); return sum; } +static unsigned int kstat_irqs(unsigned int irq) +{ + struct irq_desc *desc = irq_to_desc(irq); + + if (!desc || !desc->kstat_irqs) + return 0; + return kstat_irqs_desc(desc, cpu_possible_mask); +} + #ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT void kstat_snapshot_irqs(void) diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 6954e0a02047..5c320c3f10a7 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -488,10 +488,8 @@ int show_interrupts(struct seq_file *p, void *v) if (!desc || irq_settings_is_hidden(desc)) goto outsparse; - if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); - } + if (desc->kstat_irqs) + any_count = kstat_irqs_desc(desc, cpu_online_mask); if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) goto outsparse; -- 2.37.1 (Apple Git-137.1)
[PATCHv12 1/4] genirq: Provide a snapshot mechanism for interrupt statistics
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 Signed-off-by: Bitao Hu Reviewed-by: Liu Song Reviewed-by: Douglas Anderson --- 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 | 16 ++-- include/linux/kernel_stat.h | 8 ++ kernel/irq/Kconfig | 4 +++ kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 38 +++- kernel/irq/proc.c| 5 ++-- scripts/gdb/linux/interrupts.py | 6 ++--- 10 files changed, 66 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..fd091c35d572 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -17,6 +17,18 @@ struct irq_desc; struct irq_domain; struct pt_regs; +/** + * struct irqstat - interrupt statistics + * @cnt: real-time interrupt count + * @ref: snapshot of interrupt count + */ +struct irqstat { + unsigned intcnt; +#ifdef CONFIG_GENERIC_IRQ_STAT_SNAPSHOT + unsigned intref; +#endif +}; + /** * struct irq_desc - interrupt descriptor * @irq_common_data: per irq and chip data passed down to chip functions @@ -55,7 +67,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 intstatus_use_accessors; @@ -119,7 +131,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..9c042c6384bb 100644 --- a/include/linux/kernel_stat.h +++ b/include/linux/kernel_stat.h @@ -79,6 +79,14 @@ static inline unsigned int kstat_cpu_softirqs_sum(int cpu) return sum; } +#
[PATCHv12 0/4] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v11 to v12: - From Douglas and Thomas, add a new kconfig knob save memory when the softlock detector code is not enabled. - Adjust the order of the patches; patch #1 and patch #2 are related to genirq, while patch #3 and patch #4 are related to watchdog/softlockup, making the dependency relationships clearer. - Add the 'Reviewed-by' tag of Douglas. Changes from v10 to v11: - Only patch #2 and patch #3 have been changed. - Add comments to explain each field of 'struct irqstat' in patch #2. - Split the inner summation logic out of kstat_irqs() and encapsulate it into kstat_irqs_desc() in patch #3. - Adopt Thomas's change log for patch #3. - Add the 'Reviewed-by' tag of Liu Song. Changes from v9 to v10: - The two patches related to 'watchdog/softlockup' remain unchanged. - The majority of the work related to 'genirq' is contributed by Thomas, indicated by adding 'Originally-by' tag. And I'd like to express my gratitude for Thomas's contributions and guidance here. - Adopt Thomas's change log for the snapshot mechanism for interrupt statistics. - Split unrelated change in patch #2 into a separate patch #3. Changes from v8 to v9: - Patch #1 remains unchanged. - From Thomas Gleixner, split patch #2 into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (4): genirq: Provide a snapshot mechanism for interrupt statistics genirq: Avoid summation loops for /proc/interrupts watchdog/softlockup: low-overhead detection of interrupt storm watchdog/softlockup: report the most frequent interrupts 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 | 16 +- include/linux/kernel_stat.h | 8 + kernel/irq/Kconfig | 4 + kernel/irq/internals.h | 4 +- kernel/irq/irqdesc.c | 54 +-- kernel/irq/proc.c| 9 +- kernel/watchdog.c| 213 ++- lib/Kconfig.debug| 14 ++ scripts/gdb/linux/interrupts.py | 6 +- 12 files changed, 302 insertions(+), 32 deletions(-) -- 2.37.1 (Apple Git-137.1)
Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
Hi, On 2024/3/6 00:57, Thomas Gleixner wrote: diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index 2531f3496ab6..a28e5ac5fc79 100644 --- 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 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 This goes into the patch which adds the lockup detector parts. OK, I will implement this in the next version. Best Regards, Bitao Hu
Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
Hi, On 2024/3/4 22:24, Thomas Gleixner wrote: The above is not even configurable by the user. It's only selectable by some other config option. +# 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. That makes is visible which is pointless because it's only relevant when there is an actual user. I guess I may have misunderstood your intentions earlier. Initially, I thought you were suggesting that when "SOFTLOCKUP_DETECTOR_INTR_STORM" is not enabled, people should be able to choose "GENERIC_IRQ_STAT_SNAPSHOT" through menuconfig, so I attempted to make "GENERIC_IRQ_STAT_SNAPSHOT" visible to the user. However, after analyzing the previous emails, it seems that what you were actually proposing was to directly disable "GENERIC_IRQ_STAT_SNAPSHOT" when "SOFTLOCKUP_DETECTOR_INTR_STORM" is not enabled, as a way to save memory. If my current understanding is correct, then the code for that part would look something like the following. Does this align with your expectations? Best Regards, Bitao Hu diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index 2531f3496ab6..a28e5ac5fc79 100644 --- 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 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
Re: [PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
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 intcnt; +#ifdef CONFIG_GENIRQ_STAT_SNAPSHOT unsigned intref; +#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
[PATCHv11 4/4] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 638.870231] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [swapper/9:0] [ 638.870825] CPU#9 Utilization every 4s during lockup: [ 638.871194] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.871652] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872107] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872563] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873018] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873494] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 638.873994] #1: 330945 irq#7 [ 638.874236] #2: 31 irq#82 [ 638.874493] #3: 10 irq#10 [ 638.874744] #4: 2 irq#89 [ 638.874992] #5: 1 irq#102 ... [ 638.875313] Call trace: [ 638.875315] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu Reviewed-by: Liu Song --- kernel/watchdog.c | 115 -- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 69e72d7e461d..c9d49ae8d045 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -12,22 +12,25 @@ #define pr_fmt(fmt) "watchdog: " fmt -#include #include -#include #include +#include +#include #include +#include #include +#include #include +#include +#include #include #include + #include #include #include -#include #include -#include static DEFINE_MUTEX(watchdog_mutex); @@ -417,13 +420,104 @@ static void print_cpustat(void) } } +#define HARDIRQ_PERCENT_THRESH 50 +#define NUM_HARDIRQ_REPORT 5 +struct irq_counts { + int irq; + u32 counts; +}; + +static DEFINE_PER_CPU(bool, snapshot_taken); + +/* Tabulate the most frequent interrupts. */ +static void tabulate_irq_count(struct irq_counts *irq_counts, int irq, u32 counts, int rank) +{ + int i; + struct irq_counts new_count = {irq, counts}; + + for (i = 0; i < rank; i++) { + if (counts > irq_counts[i].counts) + swap(new_count, irq_counts[i]); + } +} + +/* + * If the hardirq time exceeds HARDIRQ_PERCENT_THRESH% of the sample_period, + * then the cause of softlockup might be interrupt storm. In this case, it + * would be useful to start interrupt counting. + */ +static bool need_counting_irqs(void) +{ + u8 util; + int tail = __this_cpu_read(cpustat_tail); + + tail = (tail + NUM_HARDIRQ_REPORT - 1) % NUM_HARDIRQ_REPORT; + util = __this_cpu_read(cpustat_util[tail][STATS_HARDIRQ]); + return util > HARDIRQ_PERCENT_THRESH; +} + +static void start_counting_irqs(void) +{ + if (!__this_cpu_read(snapshot_taken)) { + kstat_snapshot_irqs(); + __this_cpu_write(snapshot_taken, true); + } +} + +static void stop_counting_irqs(void) +{ + __this_cpu_write(snapshot_taken, false); +} + +static void print_irq_counts(void) +{ + unsigned int i, count; + struct irq_counts irq_counts_sorted[NUM_HARDIRQ_REPORT] = { + {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0} + }; + + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Detect HardIRQ Time exceeds %d%%. Most frequent HardIRQs:\n", + smp_processor_id(), HARDIRQ_PERCENT_THRESH); + + for (i = 0; i < NUM_HARDIRQ_REPORT; i++) { + if (irq_counts_sorted[i].irq == -1) + break; + + printk(KERN_CRIT "\t#%u: %-10u\tirq#%d\n", + i + 1, irq_counts_sorted[i].counts, + irq_counts_sorted[i].irq); + } + + /* +* If the hardirq time is less than HARDIRQ_PERCENT_THRESH% in the last +* sample_period, then we suspect the interrupt storm might be subsiding. +*/ + if (!need_counting_irqs()) + stop_counting_irqs(); +
[PATCHv11 3/4] genirq: Avoid summation loops for /proc/interrupts
show_interrupts() unconditionally accumulates the per CPU interrupt statistics to determine whether an interrupt was ever raised. This can be avoided for all interrupts which are not strictly per CPU and not of type NMI because those interrupts provide already an accumulated counter. The required logic is already implemented in kstat_irqs(). Split the inner access logic out of kstat_irqs() and use it for kstat_irqs() and show_interrupts() to avoid the accumulation loop when possible. Originally-by: Thomas Gleixner Signed-off-by: Bitao Hu Reviewed-by: Liu Song --- kernel/irq/internals.h | 2 ++ kernel/irq/irqdesc.c | 16 +++- kernel/irq/proc.c | 6 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index 1d92532c2aae..6c43ef3e7308 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -98,6 +98,8 @@ extern void mask_irq(struct irq_desc *desc); extern void unmask_irq(struct irq_desc *desc); extern void unmask_threaded_irq(struct irq_desc *desc); +extern unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask); + #ifdef CONFIG_SPARSE_IRQ static inline void irq_mark_irq(unsigned int irq) { } #else diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 9cd17080b2d8..65a7f2dcd17b 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -960,24 +960,30 @@ static bool irq_is_nmi(struct irq_desc *desc) return desc->istate & IRQS_NMI; } -static unsigned int kstat_irqs(unsigned int irq) +unsigned int kstat_irqs_desc(struct irq_desc *desc, const struct cpumask *cpumask) { - struct irq_desc *desc = irq_to_desc(irq); unsigned int sum = 0; int cpu; - if (!desc || !desc->kstat_irqs) - return 0; if (!irq_settings_is_per_cpu_devid(desc) && !irq_settings_is_per_cpu(desc) && !irq_is_nmi(desc)) return data_race(desc->tot_count); - for_each_possible_cpu(cpu) + for_each_cpu(cpu, cpumask) sum += data_race(per_cpu(desc->kstat_irqs->cnt, cpu)); return sum; } +static unsigned int kstat_irqs(unsigned int irq) +{ + struct irq_desc *desc = irq_to_desc(irq); + + if (!desc || !desc->kstat_irqs) + return 0; + return kstat_irqs_desc(desc, cpu_possible_mask); +} + void kstat_snapshot_irqs(void) { struct irq_desc *desc; diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 6954e0a02047..5c320c3f10a7 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -488,10 +488,8 @@ int show_interrupts(struct seq_file *p, void *v) if (!desc || irq_settings_is_hidden(desc)) goto outsparse; - if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); - } + if (desc->kstat_irqs) + any_count = kstat_irqs_desc(desc, cpu_online_mask); if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) goto outsparse; -- 2.37.1 (Apple Git-137.1)
[PATCHv11 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
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 Signed-off-by: Bitao Hu Reviewed-by: Liu Song --- 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 | 14 ++-- 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, 51 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..95d8def84471 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -17,6 +17,16 @@ struct irq_desc; struct irq_domain; struct pt_regs; +/** + * struct irqstat - interrupt statistics + * @cnt: real-time interrupt count + * @ref: snapshot of interrupt count + */ +struct irqstat { + unsigned intcnt; + unsigned intref; +}; + /** * struct irq_desc - interrupt descriptor * @irq_common_data: per irq and chip data passed down to chip functions @@ -55,7 +65,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 intstatus_use_accessors; @@ -119,7 +129,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 in
[PATCHv11 1/4] watchdog/softlockup: low-overhead detection of interrupt storm
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore,I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 98 ++- lib/Kconfig.debug | 13 +++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 81a8862295d6..69e72d7e461d 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,95 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Utilization every %llus during lockup:\n&qu
[PATCHv11 0/4] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v10 to v11: - Only patch #2 and patch #3 have been changed. - Add comments to explain each field of 'struct irqstat' in patch #2. - Split the inner summation logic out of kstat_irqs() and encapsulate it into kstat_irqs_desc() in patch #3. - Adopt Thomas's change log for patch #3. - Add the 'Reviewed-by' tag of Liu Song. Changes from v9 to v10: - The two patches related to 'watchdog/softlockup' remain unchanged. - The majority of the work related to 'genirq' is contributed by Thomas, indicated by adding 'Originally-by' tag. And I'd like to express my gratitude for Thomas's contributions and guidance here. - Adopt Thomas's change log for the snapshot mechanism for interrupt statistics. - Split unrelated change in patch #2 into a separate patch #3. Changes from v8 to v9: - Patch #1 remains unchanged. - From Thomas Gleixner, split patch #2 into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (4): watchdog/softlockup: low-overhead detection of interrupt storm genirq: Provide a snapshot mechanism for interrupt statistics genirq: Avoid summation loops for /proc/interrupts watchdog/softlockup: report the most frequent interrupts 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 | 14 +- include/linux/kernel_stat.h | 3 + kernel/irq/internals.h | 4 +- kernel/irq/irqdesc.c | 50 +-- kernel/irq/proc.c| 9 +- kernel/watchdog.c| 213 ++- lib/Kconfig.debug| 13 ++ scripts/gdb/linux/interrupts.py | 6 +- 11 files changed, 286 insertions(+), 32 deletions(-) -- 2.37.1 (Apple Git-137.1)
Re: [PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts
On 2024/2/27 23:39, Thomas Gleixner wrote: On Tue, Feb 27 2024 at 19:20, Bitao Hu wrote: On 2024/2/27 17:26, Thomas Gleixner wrote: and then let kstat_irqs() and show_interrupts() use it. See? I have a concern. kstat_irqs() uses for_each_possible_cpu() for summation. However, show_interrupts() uses for_each_online_cpu(), which means it only outputs interrupt statistics for online cpus. If we use for_each_possible_cpu() in show_interrupts() to calculate 'any_count', there could be a problem with the following scenario: If an interrupt has a count of zero on online cpus but a non-zero count on possible cpus, then 'any_count' would not be zero, and the statistics for that interrupt would be output, which is not the desired behavior for show_interrupts(). Therefore, I think it's not good to have kstat_irqs() and show_interrupts() both use the same logic. What do you think? Good point. But you simply can have unsigned int kstat_irq_desc(struct irq_desc *desc, const struct cpumask *mask) and hand in the appropriate cpumask, which still shares the code, no? Alright, that is a good approach.
Re: [PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts
Hi, On 2024/2/27 17:26, Thomas Gleixner wrote: On Mon, Feb 26 2024 at 10:09, Bitao Hu wrote: We could use the irq_desc::tot_count member to avoid the summation loop for interrupts which are not marked as 'PER_CPU' interrupts in 'show_interrupts'. This could reduce the time overhead of reading /proc/interrupts. "Could" is not really a technical term. Either we do or we do not. Also please provide context for your change and avoid the 'We'. OK. --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -121,6 +121,8 @@ static inline void irq_unlock_sparse(void) { } extern struct irq_desc irq_desc[NR_IRQS]; #endif +extern bool irq_is_nmi(struct irq_desc *desc); + If at all this wants to be in kernel/irq/internal.h. There is zero reason to expose this globally. -static bool irq_is_nmi(struct irq_desc *desc) +bool irq_is_nmi(struct irq_desc *desc) { return desc->istate & IRQS_NMI; } If at all this really wants to be a static inline in internals.h, but instead of blindly copying code this can be done smarter: unsigned int kstat_irq_desc(struct irq_desc *desc) { unsigned int sum = 0; int cpu; if (!irq_settings_is_per_cpu_devid(desc) && !irq_settings_is_per_cpu(desc) && !irq_is_nmi(desc)) return data_race(desc->tot_count); for_each_possible_cpu(cpu) sum += data_race(*per_cpu_ptr(desc->kstat_irqs, cpu)); return sum; } and then let kstat_irqs() and show_interrupts() use it. See? I have a concern. kstat_irqs() uses for_each_possible_cpu() for summation. However, show_interrupts() uses for_each_online_cpu(), which means it only outputs interrupt statistics for online cpus. If we use for_each_possible_cpu() in show_interrupts() to calculate 'any_count', there could be a problem with the following scenario: If an interrupt has a count of zero on online cpus but a non-zero count on possible cpus, then 'any_count' would not be zero, and the statistics for that interrupt would be output, which is not the desired behavior for show_interrupts(). Therefore, I think it's not good to have kstat_irqs() and show_interrupts() both use the same logic. What do you think? With that a proper changelog would be: show_interrupts() unconditionally accumulates the per CPU interrupt statistics to determine whether an interrupt was ever raised. This can be avoided for all interrupts which are not strictly per CPU and not of type NMI because those interrupts provide already an accumulated counter. The required logic is already implemented in kstat_irqs(). Split the inner access logic out of kstat_irqs() and use it for kstat_irqs() and show_interrupts() to avoid the accumulation loop when possible. Best Regards, Bitao Hu
[PATCHv10 4/4] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 638.870231] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [swapper/9:0] [ 638.870825] CPU#9 Utilization every 4s during lockup: [ 638.871194] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.871652] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872107] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872563] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873018] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873494] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 638.873994] #1: 330945 irq#7 [ 638.874236] #2: 31 irq#82 [ 638.874493] #3: 10 irq#10 [ 638.874744] #4: 2 irq#89 [ 638.874992] #5: 1 irq#102 ... [ 638.875313] Call trace: [ 638.875315] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu --- kernel/watchdog.c | 115 -- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 69e72d7e461d..c9d49ae8d045 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -12,22 +12,25 @@ #define pr_fmt(fmt) "watchdog: " fmt -#include #include -#include #include +#include +#include #include +#include #include +#include #include +#include +#include #include #include + #include #include #include -#include #include -#include static DEFINE_MUTEX(watchdog_mutex); @@ -417,13 +420,104 @@ static void print_cpustat(void) } } +#define HARDIRQ_PERCENT_THRESH 50 +#define NUM_HARDIRQ_REPORT 5 +struct irq_counts { + int irq; + u32 counts; +}; + +static DEFINE_PER_CPU(bool, snapshot_taken); + +/* Tabulate the most frequent interrupts. */ +static void tabulate_irq_count(struct irq_counts *irq_counts, int irq, u32 counts, int rank) +{ + int i; + struct irq_counts new_count = {irq, counts}; + + for (i = 0; i < rank; i++) { + if (counts > irq_counts[i].counts) + swap(new_count, irq_counts[i]); + } +} + +/* + * If the hardirq time exceeds HARDIRQ_PERCENT_THRESH% of the sample_period, + * then the cause of softlockup might be interrupt storm. In this case, it + * would be useful to start interrupt counting. + */ +static bool need_counting_irqs(void) +{ + u8 util; + int tail = __this_cpu_read(cpustat_tail); + + tail = (tail + NUM_HARDIRQ_REPORT - 1) % NUM_HARDIRQ_REPORT; + util = __this_cpu_read(cpustat_util[tail][STATS_HARDIRQ]); + return util > HARDIRQ_PERCENT_THRESH; +} + +static void start_counting_irqs(void) +{ + if (!__this_cpu_read(snapshot_taken)) { + kstat_snapshot_irqs(); + __this_cpu_write(snapshot_taken, true); + } +} + +static void stop_counting_irqs(void) +{ + __this_cpu_write(snapshot_taken, false); +} + +static void print_irq_counts(void) +{ + unsigned int i, count; + struct irq_counts irq_counts_sorted[NUM_HARDIRQ_REPORT] = { + {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0} + }; + + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Detect HardIRQ Time exceeds %d%%. Most frequent HardIRQs:\n", + smp_processor_id(), HARDIRQ_PERCENT_THRESH); + + for (i = 0; i < NUM_HARDIRQ_REPORT; i++) { + if (irq_counts_sorted[i].irq == -1) + break; + + printk(KERN_CRIT "\t#%u: %-10u\tirq#%d\n", + i + 1, irq_counts_sorted[i].counts, + irq_counts_sorted[i].irq); + } + + /* +* If the hardirq time is less than HARDIRQ_PERCENT_THRESH% in the last +* sample_period, then we suspect the interrupt storm might be subsiding. +*/ + if (!need_counting_irqs()) + stop_counting_irqs(); + } +} + static void report_cpu_status(void)
[PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts
We could use the irq_desc::tot_count member to avoid the summation loop for interrupts which are not marked as 'PER_CPU' interrupts in 'show_interrupts'. This could reduce the time overhead of reading /proc/interrupts. Originally-by: Thomas Gleixner Signed-off-by: Bitao Hu --- include/linux/irqdesc.h | 2 ++ kernel/irq/irqdesc.c| 2 +- kernel/irq/proc.c | 9 +++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h index 2912b1998670..1ee96d7232b4 100644 --- a/include/linux/irqdesc.h +++ b/include/linux/irqdesc.h @@ -121,6 +121,8 @@ static inline void irq_unlock_sparse(void) { } extern struct irq_desc irq_desc[NR_IRQS]; #endif +extern bool irq_is_nmi(struct irq_desc *desc); + static inline unsigned int irq_desc_kstat_cpu(struct irq_desc *desc, unsigned int cpu) { diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 9cd17080b2d8..56a767957a9d 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -955,7 +955,7 @@ unsigned int kstat_irqs_cpu(unsigned int irq, int cpu) return desc && desc->kstat_irqs ? per_cpu(desc->kstat_irqs->cnt, cpu) : 0; } -static bool irq_is_nmi(struct irq_desc *desc) +bool irq_is_nmi(struct irq_desc *desc) { return desc->istate & IRQS_NMI; } diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 6954e0a02047..b3b1b93f0410 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -489,8 +489,13 @@ int show_interrupts(struct seq_file *p, void *v) goto outsparse; if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); + if (!irq_settings_is_per_cpu_devid(desc) && + !irq_settings_is_per_cpu(desc) && + !irq_is_nmi(desc)) + any_count = data_race(desc->tot_count); + else + for_each_online_cpu(j) + any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); } if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) -- 2.37.1 (Apple Git-137.1)
[PATCHv10 2/4] genirq: Provide a snapshot mechanism for interrupt statistics
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 Signed-off-by: Bitao Hu --- 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 intcnt; + unsigned intref; +}; + /** * 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 intstatus_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/
[PATCHv10 1/4] watchdog/softlockup: low-overhead detection of interrupt storm
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore,I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 98 ++- lib/Kconfig.debug | 13 +++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 81a8862295d6..69e72d7e461d 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,95 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Utilization every %llus during lockup:\n&qu
[PATCHv10 0/4] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v9 to v10: - The two patches related to 'watchdog/softlockup' remain unchanged. - The majority of the work related to 'genirq' is contributed by Thomas, indicated by adding 'Originally-by' tag. And I'd like to express my gratitude for Thomas's contributions and guidance here. - Adopt Thomas's change log for the snapshot mechanism for interrupt statistics. - Split unrelated change in patch #2 into a separate patch #3. Changes from v8 to v9: - Patch #1 remains unchanged. - From Thomas Gleixner, split patch #2 into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (4): watchdog/softlockup: low-overhead detection of interrupt storm genirq: Provide a snapshot mechanism for interrupt statistics genirq: Avoid summation loops for /proc/interrupts watchdog/softlockup: report the most frequent interrupts 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 | 11 +- include/linux/kernel_stat.h | 3 + kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 36 - kernel/irq/proc.c| 12 +- kernel/watchdog.c| 213 ++- lib/Kconfig.debug| 13 ++ scripts/gdb/linux/interrupts.py | 6 +- 11 files changed, 276 insertions(+), 26 deletions(-) -- 2.37.1 (Apple Git-137.1)
Re: [PATCHv9 2/3] irq: use a struct for the kstat_irqs in the interrupt descriptor
Hi, On 2024/2/22 21:22, Thomas Gleixner wrote: On Thu, Feb 22 2024 at 17:34, Bitao Hu wrote: First of all the subsystem prefix is 'genirq:'. 'git log kernel/irq/' gives you a pretty good hint. It's documented Secondly the subject line does not match what this patch is about. It's not about using a struct, it's about providing a snapshot mechanism, no? The current implementation uses an int for the kstat_irqs in the interrupt descriptor. However, we need to know the number of interrupts which happened since softlockup detection took a snapshot in order to analyze the problem caused by an interrupt storm. Replacing an int with a struct and providing sensible interfaces for the watchdog code can keep it self contained to the interrupt core code. So something like this makes a useful change log for this: Subject: genirq: Provide a snapshot mechanism for interrupt statistics 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. Thanks, the changelog you wrote very clearly articulates the purpose of this patch. Hmm? Signed-off-by: Bitao Hu Interesting. You fully authored the patch? That's not how it works. You cannot take work from others and claim that it is yours. The minimal courtesy is to add a 'Originally-by:' tag. I'm very sorry, the majority of this patch is your work, I will add an 'Originally-by:' tag. diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 623b8136e9af..3ad40cf30c66 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -488,18 +488,15 @@ int show_interrupts(struct seq_file *p, void *v) if (!desc || irq_settings_is_hidden(desc)) goto outsparse; - if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(*per_cpu_ptr(desc->kstat_irqs, j)); - } + if (desc->kstat_irqs) + any_count = data_race(desc->tot_count); This is an unrelated change and needs to be split out into a separate patch with a proper changelog which explains why this is equivalent. Alright, I will remove this change witch is not related to the purpose of this patch. I guess that the purpose of suggesting this change in your V1 response was to speedup the 'show_interrupts'. However, after reviewing the usage of 'desc->tot_count' in 'unsigned int kstat_irqs(unsigned int irq)', I think the change might be as follows: diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c index 623b8136e9af..53b8d6edd7ac 100644 --- a/kernel/irq/proc.c +++ b/kernel/irq/proc.c @@ -489,8 +489,13 @@ int show_interrupts(struct seq_file *p, void *v) goto outsparse; if (desc->kstat_irqs) { - for_each_online_cpu(j) - any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); + if (!irq_settings_is_per_cpu_devid(desc) && + !irq_settings_is_per_cpu(desc) && + !irq_is_nmi(desc)) + any_count = data_race(desc->tot_count); + else + for_each_online_cpu(j) + any_count |= data_race(per_cpu(desc->kstat_irqs->cnt, j)); } if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) Is my idea correct? Best Regards, Bitao
[PATCHv9 3/3] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 638.870231] watchdog: BUG: soft lockup - CPU#9 stuck for 26s! [swapper/9:0] [ 638.870825] CPU#9 Utilization every 4s during lockup: [ 638.871194] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.871652] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872107] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.872563] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873018] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 638.873494] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 638.873994] #1: 330945 irq#7 [ 638.874236] #2: 31 irq#82 [ 638.874493] #3: 10 irq#10 [ 638.874744] #4: 2 irq#89 [ 638.874992] #5: 1 irq#102 ... [ 638.875313] Call trace: [ 638.875315] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu --- kernel/watchdog.c | 115 -- 1 file changed, 111 insertions(+), 4 deletions(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 69e72d7e461d..c9d49ae8d045 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -12,22 +12,25 @@ #define pr_fmt(fmt) "watchdog: " fmt -#include #include -#include #include +#include +#include #include +#include #include +#include #include +#include +#include #include #include + #include #include #include -#include #include -#include static DEFINE_MUTEX(watchdog_mutex); @@ -417,13 +420,104 @@ static void print_cpustat(void) } } +#define HARDIRQ_PERCENT_THRESH 50 +#define NUM_HARDIRQ_REPORT 5 +struct irq_counts { + int irq; + u32 counts; +}; + +static DEFINE_PER_CPU(bool, snapshot_taken); + +/* Tabulate the most frequent interrupts. */ +static void tabulate_irq_count(struct irq_counts *irq_counts, int irq, u32 counts, int rank) +{ + int i; + struct irq_counts new_count = {irq, counts}; + + for (i = 0; i < rank; i++) { + if (counts > irq_counts[i].counts) + swap(new_count, irq_counts[i]); + } +} + +/* + * If the hardirq time exceeds HARDIRQ_PERCENT_THRESH% of the sample_period, + * then the cause of softlockup might be interrupt storm. In this case, it + * would be useful to start interrupt counting. + */ +static bool need_counting_irqs(void) +{ + u8 util; + int tail = __this_cpu_read(cpustat_tail); + + tail = (tail + NUM_HARDIRQ_REPORT - 1) % NUM_HARDIRQ_REPORT; + util = __this_cpu_read(cpustat_util[tail][STATS_HARDIRQ]); + return util > HARDIRQ_PERCENT_THRESH; +} + +static void start_counting_irqs(void) +{ + if (!__this_cpu_read(snapshot_taken)) { + kstat_snapshot_irqs(); + __this_cpu_write(snapshot_taken, true); + } +} + +static void stop_counting_irqs(void) +{ + __this_cpu_write(snapshot_taken, false); +} + +static void print_irq_counts(void) +{ + unsigned int i, count; + struct irq_counts irq_counts_sorted[NUM_HARDIRQ_REPORT] = { + {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0}, {-1, 0} + }; + + if (__this_cpu_read(snapshot_taken)) { + for_each_active_irq(i) { + count = kstat_get_irq_since_snapshot(i); + tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT); + } + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Detect HardIRQ Time exceeds %d%%. Most frequent HardIRQs:\n", + smp_processor_id(), HARDIRQ_PERCENT_THRESH); + + for (i = 0; i < NUM_HARDIRQ_REPORT; i++) { + if (irq_counts_sorted[i].irq == -1) + break; + + printk(KERN_CRIT "\t#%u: %-10u\tirq#%d\n", + i + 1, irq_counts_sorted[i].counts, + irq_counts_sorted[i].irq); + } + + /* +* If the hardirq time is less than HARDIRQ_PERCENT_THRESH% in the last +* sample_period, then we suspect the interrupt storm might be subsiding. +*/ + if (!need_counting_irqs()) + stop_counting_irqs(); + } +} + static void report_cpu_status(void)
[PATCHv9 2/3] irq: use a struct for the kstat_irqs in the interrupt descriptor
The current implementation uses an int for the kstat_irqs in the interrupt descriptor. However, we need to know the number of interrupts which happened since softlockup detection took a snapshot in order to analyze the problem caused by an interrupt storm. Replacing an int with a struct and providing sensible interfaces for the watchdog code can keep it self contained to the interrupt core code. Signed-off-by: Bitao Hu --- 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| 9 +++- scripts/gdb/linux/interrupts.py | 6 ++--- 9 files changed, 47 insertions(+), 22 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 intcnt; + unsigned intref; +}; + /** * 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 intstatus_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
[PATCHv9 1/3] watchdog/softlockup: low-overhead detection of interrupt storm
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore,I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 98 ++- lib/Kconfig.debug | 13 +++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 81a8862295d6..69e72d7e461d 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,95 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Utilization every %llus during lockup:\n&qu
[PATCHv9 0/3] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v8 to v9: - Patch #1 remains unchanged. - From Thomas Gleixner, split patch #2 into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (3): watchdog/softlockup: low-overhead detection of interrupt storm irq: use a struct for the kstat_irqs in the interrupt descriptor watchdog/softlockup: report the most frequent interrupts 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| 9 +- kernel/watchdog.c| 213 ++- lib/Kconfig.debug| 13 ++ scripts/gdb/linux/interrupts.py | 6 +- 11 files changed, 268 insertions(+), 27 deletions(-) -- 2.37.1 (Apple Git-137.1)
Re: [PATCHv8 2/2] watchdog/softlockup: report the most frequent interrupts
Hi, On 2024/2/20 17:35, Thomas Gleixner wrote: On Tue, Feb 20 2024 at 00:19, Bitao Hu wrote: 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 | 4 + kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 34 ++-- kernel/irq/proc.c| 9 +-- This really wants to be split into two patches. Interrupt infrastructure first and then the actual usage site in the watchdog code. Okay, I will split it into two patches.
[PATCHv8 1/2] watchdog/softlockup: low-overhead detection of interrupt
The following softlockup is caused by interrupt storm, but it cannot be identified from the call tree. Because the call tree is just a snapshot and doesn't fully capture the behavior of the CPU during the soft lockup. watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] ... Call trace: __do_softirq+0xa0/0x37c __irq_exit_rcu+0x108/0x140 irq_exit+0x14/0x20 __handle_domain_irq+0x84/0xe0 gic_handle_irq+0x80/0x108 el0_irq_naked+0x50/0x58 Therefore,I think it is necessary to report CPU utilization during the softlockup_thresh period (report once every sample_period, for a total of 5 reportings), like this: watchdog: BUG: soft lockup - CPU#28 stuck for 23s! [fio:83921] CPU#28 Utilization every 4s during lockup: #1: 0% system, 0% softirq, 100% hardirq, 0% idle #2: 0% system, 0% softirq, 100% hardirq, 0% idle #3: 0% system, 0% softirq, 100% hardirq, 0% idle #4: 0% system, 0% softirq, 100% hardirq, 0% idle #5: 0% system, 0% softirq, 100% hardirq, 0% idle ... This would be helpful in determining whether an interrupt storm has occurred or in identifying the cause of the softlockup. The criteria for determination are as follows: a. If the hardirq utilization is high, then interrupt storm should be considered and the root cause cannot be determined from the call tree. b. If the softirq utilization is high, then we could analyze the call tree but it may cannot reflect the root cause. c. If the system utilization is high, then we could analyze the root cause from the call tree. The mechanism requires a considerable amount of global storage space when configured for the maximum number of CPUs. Therefore, adding a SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob that defaults to "yes" if the max number of CPUs is <= 128. Signed-off-by: Bitao Hu Reviewed-by: Douglas Anderson Reviewed-by: Liu Song --- kernel/watchdog.c | 98 ++- lib/Kconfig.debug | 13 +++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 81a8862295d6..69e72d7e461d 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -35,6 +37,8 @@ static DEFINE_MUTEX(watchdog_mutex); # define WATCHDOG_HARDLOCKUP_DEFAULT 0 #endif +#define NUM_SAMPLE_PERIODS 5 + unsigned long __read_mostly watchdog_enabled; int __read_mostly watchdog_user_enabled = 1; static int __read_mostly watchdog_hardlockup_user_enabled = WATCHDOG_HARDLOCKUP_DEFAULT; @@ -333,6 +337,95 @@ __setup("watchdog_thresh=", watchdog_thresh_setup); static void __lockup_detector_cleanup(void); +#ifdef CONFIG_SOFTLOCKUP_DETECTOR_INTR_STORM +enum stats_per_group { + STATS_SYSTEM, + STATS_SOFTIRQ, + STATS_HARDIRQ, + STATS_IDLE, + NUM_STATS_PER_GROUP, +}; + +static const enum cpu_usage_stat tracked_stats[NUM_STATS_PER_GROUP] = { + CPUTIME_SYSTEM, + CPUTIME_SOFTIRQ, + CPUTIME_IRQ, + CPUTIME_IDLE, +}; + +static DEFINE_PER_CPU(u16, cpustat_old[NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_util[NUM_SAMPLE_PERIODS][NUM_STATS_PER_GROUP]); +static DEFINE_PER_CPU(u8, cpustat_tail); + +/* + * We don't need nanosecond resolution. A granularity of 16ms is + * sufficient for our precision, allowing us to use u16 to store + * cpustats, which will roll over roughly every ~1000 seconds. + * 2^24 ~= 16 * 10^6 + */ +static u16 get_16bit_precision(u64 data_ns) +{ + return data_ns >> 24LL; /* 2^24ns ~= 16.8ms */ +} + +static void update_cpustat(void) +{ + int i; + u8 util; + u16 old_stat, new_stat; + struct kernel_cpustat kcpustat; + u64 *cpustat = kcpustat.cpustat; + u8 tail = __this_cpu_read(cpustat_tail); + u16 sample_period_16 = get_16bit_precision(sample_period); + + kcpustat_cpu_fetch(&kcpustat, smp_processor_id()); + + for (i = 0; i < NUM_STATS_PER_GROUP; i++) { + old_stat = __this_cpu_read(cpustat_old[i]); + new_stat = get_16bit_precision(cpustat[tracked_stats[i]]); + util = DIV_ROUND_UP(100 * (new_stat - old_stat), sample_period_16); + __this_cpu_write(cpustat_util[tail][i], util); + __this_cpu_write(cpustat_old[i], new_stat); + } + + __this_cpu_write(cpustat_tail, (tail + 1) % NUM_SAMPLE_PERIODS); +} + +static void print_cpustat(void) +{ + int i, group; + u8 tail = __this_cpu_read(cpustat_tail); + u64 sample_period_second = sample_period; + + do_div(sample_period_second, NSEC_PER_SEC); + + /* +* We do not want the "watchdog: " prefix on every line, +* hence we use "printk" instead of "pr_crit". +*/ + printk(KERN_CRIT "CPU#%d Utilization every %llus during lockup:\n&qu
[PATCHv8 0/2] *** Detect interrupt storm in softlockup ***
Hi, guys. I have implemented a low-overhead method for detecting interrupt storm in softlockup. Please review it, all comments are welcome. Changes from v7 to v8: - From Thomas Gleixner, implement statistics within the interrupt core code and provide sensible interfaces for the watchdog code. - Patch #1 remains unchanged. Patch #2 has significant changes based on Thomas's suggestions, which is why I have removed Liu Song and Douglas's Reviewed-by from patch #2. Please review it again, and all comments are welcome. Changes from v6 to v7: - Remove "READ_ONCE" in "start_counting_irqs" - Replace the hard-coded 5 with "NUM_SAMPLE_PERIODS" macro in "set_sample_period". - Add empty lines to help with reading the code. - Remove the branch that processes IRQs where "counts_diff = 0". - Add the Reviewed-by of Liu Song and Douglas. Changes from v5 to v6: - Use "./scripts/checkpatch.pl --strict" to get a few extra style nits and fix them. - Squash patch #3 into patch #1, and wrapp the help text to 80 columns. - Sort existing headers alphabetically in watchdog.c - Drop "softlockup_hardirq_cpus", just read "hardirq_counts" and see if it's non-NULL. - Store "nr_irqs" in a local variable. - Simplify the calculation of "cpu_diff". Changes from v4 to v5: - Rearranging variable placement to make code look neater. Changes from v3 to v4: - Renaming some variable and function names to make the code logic more readable. - Change the code location to avoid predeclaring. - Just swap rather than a double loop in tabulate_irq_count. - Since nr_irqs has the potential to grow at runtime, bounds-check logic has been implemented. - Add SOFTLOCKUP_DETECTOR_INTR_STORM Kconfig knob. Changes from v2 to v3: - From Liu Song, using enum instead of macro for cpu_stats, shortening the name 'idx_to_stat' to 'stats', adding 'get_16bit_precesion' instead of using right shift operations, and using 'struct irq_counts'. - From kernel robot test, using '__this_cpu_read' and '__this_cpu_write' instead of accessing to an per-cpu array directly, in order to avoid this warning. 'sparse: incorrect type in initializer (different modifiers)' Changes from v1 to v2: - From Douglas, optimize the memory of cpustats. With the maximum number of CPUs, that's now this. 2 * 8192 * 4 + 1 * 8192 * 5 * 4 + 1 * 8192 = 237,568 bytes. - From Liu Song, refactor the code format and add necessary comments. - From Douglas, use interrupt counts instead of interrupt time to determine the cause of softlockup. - Remove the cmdline parameter added in PATCHv1. Bitao Hu (2): watchdog/softlockup: low-overhead detection of interrupt watchdog/softlockup: report the most frequent interrupts 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 | 4 + kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 34 - kernel/irq/proc.c| 9 +- kernel/watchdog.c| 213 ++- lib/Kconfig.debug| 13 ++ scripts/gdb/linux/interrupts.py | 6 +- 11 files changed, 269 insertions(+), 27 deletions(-) -- 2.37.1 (Apple Git-137.1)
[PATCHv8 2/2] watchdog/softlockup: report the most frequent interrupts
When the watchdog determines that the current soft lockup is due to an interrupt storm based on CPU utilization, reporting the most frequent interrupts could be good enough for further troubleshooting. Below is an example of interrupt storm. The call tree does not provide useful information, but we can analyze which interrupt caused the soft lockup by comparing the counts of interrupts. [ 2987.488075] watchdog: BUG: soft lockup - CPU#9 stuck for 23s! [kworker/9:1:214] [ 2987.488607] CPU#9 Utilization every 4s during lockup: [ 2987.488941] #1: 0% system, 0% softirq, 100% hardirq, 0% idle [ 2987.489357] #2: 0% system, 0% softirq, 100% hardirq, 0% idle [ 2987.489771] #3: 0% system, 0% softirq, 100% hardirq, 0% idle [ 2987.490186] #4: 0% system, 0% softirq, 100% hardirq, 0% idle [ 2987.490601] #5: 0% system, 0% softirq, 100% hardirq, 0% idle [ 2987.491034] CPU#9 Detect HardIRQ Time exceeds 50%. Most frequent HardIRQs: [ 2987.491493] #1: 330985 irq#7 [ 2987.491743] #2: 5000irq#10 [ 2987.492039] #3: 9 irq#91 [ 2987.492318] #4: 3 irq#118 ... [ 2987.492728] Call trace: [ 2987.492729] __do_softirq+0xa8/0x364 Signed-off-by: Bitao Hu --- 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 | 4 + kernel/irq/internals.h | 2 +- kernel/irq/irqdesc.c | 34 ++-- kernel/irq/proc.c| 9 +-- kernel/watchdog.c| 115 ++- scripts/gdb/linux/interrupts.py | 6 +- 10 files changed, 159 insertions(+), 26 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 intcnt; + unsigned intref; +}; + /** * 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 intstatus_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..9cbb1361f957 100644 --- a/include/linux/ke