Re: [PATCH 4/6] x86-mce: Add spinlocks to prevent duplicated MCP and CMCI reports.

[Borislav Petkov <bp@alien8.de>]

On Fri, Jul 11, 2014 at 12:06:40PM -0700, Tony Luck wrote:
> > +               if (atomic_add_unless(&mce_banks[i].poll_reader, 1, 1)) {
> > +                       m.status = mce_rdmsrl(MSR_IA32_MCx_STATUS(i));
> 
> Same as yesterday. You may skip reading a bank because someone else
> is reading the same bank number, even though you don't share that bank
> with them.

Not if those banks are in a percpu variable. And this is what
machine_check_poll gets. The ->poll_reader thing is then per-cpu too.

For shared banks it should work also as expected since we want there
only one reader to see the MCE signature.

> If we are willing to be rather flexible amount when polling happens,
> and not allow very fast poll rates. Then we could do something like
> have the lowest numbered online cpu be the only one that sets a
> timer. When it goes off, it scans its own banks, and then uses an
> async cross-processor call to poke the next highest numbered
> online cpu to have it scan banks and poke the next guy.
> 
> That way we know that two cpus can't be polling at the same time,
> because we convoy them all one at a time.

See above - those banks are percpu. And besides, mce_timer_fn already
has the WARN_ON which otherwise be firing left and right.

It seems, Havard's issue is only with shared banks. I think they only
cause the repeated error records.

> Fast poll rates would be a problem on very large systems. Might
> need to have the highest numbered cpu notice that it is at the
> end of the chain and set some flag so the lowest one can tell
> whether it is safe to begin the next ripple.

Well, if fast polling rates will be a problem anyway, we probably should
talk about adjusting the polling alg. too.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--