Re: [PATCH v4 2/2] x86/mce: Avoid arming periodic polling timer when not required

[Aaron Tomlin]

On Thu, Sep 03, 2026 at 10:49:34AM +0200, Marco Crivellari wrote:
> Hi Aaron,
> 
> On Thu, Sep 3, 2026 at 6:13 AM Aaron Tomlin <atomlin@xxxxxxxxxxx> wrote:
> >
> > +++ b/arch/x86/kernel/cpu/mce/intel.c
> > @@ -229,16 +229,17 @@ static u64 cmci_pick_threshold(u64 val, int *bios_zero_thresh)
> >   */
> >  static void cmci_claim_bank(int bank, u64 val, int bios_zero_thresh, int *bios_wrong_thresh)
> >  {
> > -       struct mca_storm_desc *storm = this_cpu_ptr(&storm_desc);
> > -
> >         val |= MCI_CTL2_CMCI_EN;
> >         wrmsrq(MSR_IA32_MCx_CTL2(bank), val);
> >         rdmsrq(MSR_IA32_MCx_CTL2(bank), val);
> >
> > -       /* If the enable bit did not stick, this bank should be polled. */
> > +       /*
> > +        * If the enable bit did not stick, this bank does not support CMCI
> > +        * and no corrected or UCNA errors will be reported on this bank
> > +        * (SDM Vol 3B 18.5). No polling is needed.
> > +        */
> >         if (!(val & MCI_CTL2_CMCI_EN)) {
> > -               WARN_ON(!test_bit(bank, this_cpu_ptr(mce_poll_banks)));
> > -               storm->banks[bank].poll_only = true;
> > +               __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
> >                 return;
> >         }
> 
> Not sure Sashiko's review it makes sense but to me it could make sense [1].
> In short:
> 
> > cmci_storm_begin()
> >    __set_bit(bank, this_cpu_ptr(mce_poll_banks));
> > cmci_storm_end()
> >    __clear_bit(bank, this_cpu_ptr(mce_poll_banks));
> >
> > Since __clear_bit() and __set_bit() are non-atomic read-modify-write operations
> > and softirqs run with hardirqs enabled, will the hardirq's modification be
> > overwritten and lost when the softirq resumes and writes back its value?
> 
> Sashiko's suggestion is to use the atomic versions, "clear_bit()" and
> "set_bit()".
> 
> Thanks!
> 
> - [1] https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com
> 
> -- 
> 
> Marco Crivellari
> 
> SUSE Labs

Hi Marco,

Thank you for your review!

Yes, the per-CPU variable mce_poll_banks can indeed be modified across
different execution contexts on the same CPU.

Tony, I will address what Sashiko [1] reported in the next iteration.

[1]: https://sashiko.dev/#/patchset/20260903041320.179965-1-atomlin%40atomlin.com

Kind regards,
-- 
Aaron Tomlin