lkml.org 
[lkml]   [2020]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v3 16/16] ARM: Remove custom IRQ stat accounting
    From
    Date
    Hi Marc,

    On 01/09/2020 15:43, Marc Zyngier wrote:
    > Let's switch the arm code to the core accounting, which already
    > does everything we need.
    >
    > Reviewed-by: Valentin Schneider <valentin.schneider@arm.com>
    > Signed-off-by: Marc Zyngier <maz@kernel.org>
    > ---
    > arch/arm/include/asm/hardirq.h | 17 -----------------
    > arch/arm/kernel/smp.c | 20 ++++----------------
    > 2 files changed, 4 insertions(+), 33 deletions(-)

    This appears to be causing a NULL pointer dereference on
    beaglebone-black, it got bisected automatically several times.
    None of the other platforms in the KernelCI labs appears to be
    affected.

    Here's the error in the full job log, with next-20200923:

    https://storage.staging.kernelci.org/kernelci/staging.kernelci.org/staging-20200924.0/arm/multi_v7_defconfig/gcc-8/lab-baylibre/baseline-beaglebone-black.html#L460

    and some meta-data:

    https://staging.kernelci.org/test/case/id/5f6bea67f724eb1b34dce584/

    The full bisection report is available here:

    https://groups.io/g/kernelci-results-staging/message/2094

    I've also run it again with a debug build to locate the problem,
    see below.


    > diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
    > index 7a88f160b1fb..b95848ed2bc7 100644
    > --- a/arch/arm/include/asm/hardirq.h
    > +++ b/arch/arm/include/asm/hardirq.h
    > @@ -6,29 +6,12 @@
    > #include <linux/threads.h>
    > #include <asm/irq.h>
    >
    > -/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
    > -#define NR_IPI 7
    > -
    > typedef struct {
    > unsigned int __softirq_pending;
    > -#ifdef CONFIG_SMP
    > - unsigned int ipi_irqs[NR_IPI];
    > -#endif
    > } ____cacheline_aligned irq_cpustat_t;
    >
    > #include <linux/irq_cpustat.h> /* Standard mappings for irq_cpustat_t above */
    >
    > -#define __inc_irq_stat(cpu, member) __IRQ_STAT(cpu, member)++
    > -#define __get_irq_stat(cpu, member) __IRQ_STAT(cpu, member)
    > -
    > -#ifdef CONFIG_SMP
    > -u64 smp_irq_stat_cpu(unsigned int cpu);
    > -#else
    > -#define smp_irq_stat_cpu(cpu) 0
    > -#endif
    > -
    > -#define arch_irq_stat_cpu smp_irq_stat_cpu
    > -
    > #define __ARCH_IRQ_EXIT_IRQS_DISABLED 1
    >
    > #endif /* __ASM_HARDIRQ_H */
    > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
    > index d51e64955a26..aead847ac8b9 100644
    > --- a/arch/arm/kernel/smp.c
    > +++ b/arch/arm/kernel/smp.c
    > @@ -65,6 +65,7 @@ enum ipi_msg_type {
    > IPI_CPU_STOP,
    > IPI_IRQ_WORK,
    > IPI_COMPLETION,
    > + NR_IPI,
    > /*
    > * CPU_BACKTRACE is special and not included in NR_IPI
    > * or tracable with trace_ipi_*
    > @@ -529,27 +530,16 @@ void show_ipi_list(struct seq_file *p, int prec)
    > unsigned int cpu, i;
    >
    > for (i = 0; i < NR_IPI; i++) {
    > + unsigned int irq = irq_desc_get_irq(ipi_desc[i]);

    It looks like irq_desc_get_irq() gets called with a NULL
    pointer (well, 0x0000001c):

    (gdb) l *0xc030ef38
    0xc030ef38 is in show_ipi_list (../include/linux/irqdesc.h:123).
    118 return container_of(data->common, struct irq_desc, irq_common_data);
    119 }
    120
    121 static inline unsigned int irq_desc_get_irq(struct irq_desc *desc)
    122 {
    123 return desc->irq_data.irq;
    124 }
    125
    126 static inline struct irq_data *irq_desc_get_irq_data(struct irq_desc *desc)
    127 {

    Full job log: https://lava.baylibre.com/scheduler/job/142375#L727

    I haven't looked any further but hopefully this should be a good
    enough clue to find the root cause. I don't know if you have a
    platform at hand to reproduce the issue, please let me know if
    you need some help with debugging or testing a fix.

    Hope this helps,
    Guillaume


    > seq_printf(p, "%*s%u: ", prec - 1, "IPI", i);
    >
    > for_each_online_cpu(cpu)
    > - seq_printf(p, "%10u ",
    > - __get_irq_stat(cpu, ipi_irqs[i]));
    > + seq_printf(p, "%10u ", kstat_irqs_cpu(irq, cpu));
    >
    > seq_printf(p, " %s\n", ipi_types[i]);
    > }
    > }
    >
    > -u64 smp_irq_stat_cpu(unsigned int cpu)
    > -{
    > - u64 sum = 0;
    > - int i;
    > -
    > - for (i = 0; i < NR_IPI; i++)
    > - sum += __get_irq_stat(cpu, ipi_irqs[i]);
    > -
    > - return sum;
    > -}
    > -
    > void arch_send_call_function_ipi_mask(const struct cpumask *mask)
    > {
    > smp_cross_call(mask, IPI_CALL_FUNC);
    > @@ -630,10 +620,8 @@ static void do_handle_IPI(int ipinr)
    > {
    > unsigned int cpu = smp_processor_id();
    >
    > - if ((unsigned)ipinr < NR_IPI) {
    > + if ((unsigned)ipinr < NR_IPI)
    > trace_ipi_entry_rcuidle(ipi_types[ipinr]);
    > - __inc_irq_stat(cpu, ipi_irqs[ipinr]);
    > - }
    >
    > switch (ipinr) {
    > case IPI_WAKEUP:
    >

    \
     
     \ /
      Last update: 2020-09-24 11:00    [W:4.065 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site