Re: [RFC PATCH] mce: prevent concurrent polling of MCE events

[Aristeu Rozanski <aris@ruivo.org>]

Hi Tony,

On Tue, Jan 17, 2023 at 05:42:36PM +0000, Luck, Tony wrote:
> > +static DEFINE_RAW_SPINLOCK(timer_fn_lock);
> >
> >  static unsigned long mce_adjust_timer_default(unsigned long interval)
> >  {
> > @@ -1628,7 +1629,9 @@ static void mce_timer_fn(struct timer_list *t)
> >       iv = __this_cpu_read(mce_next_interval);
> >
> >       if (mce_available(this_cpu_ptr(&cpu_info))) {
> > +             raw_spin_lock(&timer_fn_lock);
> >               machine_check_poll(0, this_cpu_ptr(&mce_poll_banks));
> > +             raw_spin_unlock(&timer_fn_lock);
> >
> >               if (mce_intel_cmci_poll()) {
> >                       iv = mce_adjust_timer(iv);
> 
> If the CMCI polling interrupts on those large number of CPUs are
> staggered at different times, then this should be fine. But if a largish
> number of CPUs get in lockstep with each other, then this could get
> ugly.  I.e. 200 CPUs all take the "CMCI poll" interrupt together. Then
> get stuck in a convoy here as one at a time step through checking
> the machine check banks.
> 
> I'm not sure if that is just a theoretical issue, or how bad it might
> actually be if it happened.
> 
> One option to avoid this might be to change from a fixed five minute
> polling interval to "five minute plus/minus rand(50) jiffies". Then even
> if some CPUs did sync up, they'd quickly diverge again.

I did experiment different ranges including forcing a minimum difference
based on cpu number but eventually I'd see repeated events. Perhaps a
combination would be acceptable? Set the first run of each cpu spaced based on
CPU number then use a spinlock to synchronize it? That way we minimize the
chance of a big number of CPUs stuck on the same lock but at same time
guarantee we won't have duplicated events.

-- 
Aristeu