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

[Tony Luck <tony.luck@intel.com>]

On Fri, Apr 28, 2023 at 04:43:46PM +0000, Luck, Tony wrote:
> > 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,
> 
> Yes. A variable poll interval (to avoid CPUs all bunching together) with
> the spinlock to serialize the cases where some CPUs do line up their
> polling would work.

I just ran a test on a system booted with mce=no_cmci and saw the
duplicate reporting.  Digging into the code I see that I'm not the
first to think that a little randomness in timers on different CPUs
would be a good idea. The MCE code starts/restarts timers with:

	mod_timer(t, round_jiffies(when));

Looking at the round_jiffies() function I sw that it is grabbing
the current CPU number.

unsigned long round_jiffies(unsigned long j)
{
        return round_jiffies_common(j, raw_smp_processor_id(), false);
}

And round_jiffies_common() explains with this comment:

        /*
         * We don't want all cpus firing their timers at once hitting the
         * same lock or cachelines, so we skew each extra cpu with an extra
         * 3 jiffies. This 3 jiffies came originally from the mm/ code which
         * already did this.
         * The skew is done by adding 3*cpunr, then round, then subtract this
         * extra offset again.
         */
        j += cpu * 3;

	rem = j % HZ;

So your original patch https://lore.kernel.org/all/Y8WtE7BNJ0gTrqIS@cathedrallabs.org/
that just added the spinlock should be fine!

-Tony