Messages in this thread | | | Date | Sun, 11 Feb 2024 23:41:23 +0800 | Subject | Re: [PATCHv6 2/2] watchdog/softlockup: report the most frequent interrupts | From | Bitao Hu <> |
| |
On 2024/2/9 00:03, Doug Anderson wrote: > Hi, > > On Thu, Feb 8, 2024 at 4:54 AM Bitao Hu <yaoma@linux.alibaba.com> 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. Oh, yes, READ_ONCE() is not necessary here. > > 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@chromium.org>
| |