Re: [PATCHv6 2/2] watchdog/softlockup: report the most frequent interrupts

[Doug Anderson]

Hi,

On Thu, Feb 8, 2024 at 4:54 AM Bitao Hu <yaoma@xxxxxxxxxxxxxxxxx> wrote:
>
> +static void start_counting_irqs(void)
> +{
> +       int i;
> +       int local_nr_irqs;
> +       struct irq_desc *desc;
> +       u32 *counts = __this_cpu_read(hardirq_counts);
> +
> +       if (!counts) {
> +               /*
> +                * nr_irqs has the potential to grow at runtime. We should read
> +                * it and store locally to avoid array out-of-bounds access.
> +                */
> +               local_nr_irqs = READ_ONCE(nr_irqs);

nit: I don't think the READ_ONCE() is actually needed above. All that
matters is that you're consistently using the same local variable
("local_nr_irqs") for allocating the array, looping, and then storing.
No matter what optimizations might be happening and what else might be
happening on other CPUs, once you put it in a local variable the
compiler _must_ keep it consistent.

That being said, I don't think it really matters, so I'm not sure it's
worth spinning your series just for that.

In any case, this patch looks good to me now. Thanks!

Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>