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 02:15:49PM -0700, Havard Skinnemoen wrote:
> But the problem with shared banks is that multiple CPUs read them,
> and we can't tell if the errors are duplicate unless we make sure
> that only one CPU gets to read and clear it at a time. Any cpu-local
> synchronization mechanism isn't going to work.

Well, maybe it is about time we tracked shared banks.

> If you're worried about disabling interrupts, it's possible we don't
> really need to make the spinlocks irqsafe. I'm not sure if we had any
> reason for that other than "just to be safe".

Hmm, so machine_check_poll gets called mostly in irqs off context except
in the timer callback which runs in softirq context.

I hear hrtimer callbacks will be made to run *always* in interrupt
context so we could switch cmci to use an hrtimer at some point...

> Or we could keep the (irqsafe) spinlocks but move the clearing much
> earlier. There may have been a reason why the current code clears the
> bank status last though -- perhaps we also need to read out all the
> state while we hold the lock, before we clear the status bit.

... or, yeah, do what you do currently and disable IRQs but do all the
MSR accesses in there, after you've detected MCI_STATUS_VAL set. That
would make the critical section at least shorter even if we disable
interrupts for non-shared banks too.

The clearing of the MCI_STATUS_VAL bit is unrelated to the rest of the
MSRs in the MCA bank so you can read them all in one go in the critical
section.

We can evaluate later if the IRQs disabling is too heavy after all.

-- 
Regards/Gruss,
    Boris.

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