Re: [PATCH 2/2] edac: add support for Amazon's Annapurna Labs EDAC

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Wed, 12 Jun 2019 18:29:26 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> escreveu:

> On Wed, 2019-06-12 at 05:48 +0200, Borislav Petkov wrote:
> > On Wed, Jun 12, 2019 at 08:25:52AM +1000, Benjamin Herrenschmidt wrote:  
> > > Yes, we would be in a world of pain already if tracepoints couldn't
> > > handle concurrency :-)  
> > 
> > Right, lockless buffer and the whole shebang :)  
> 
> Yup.
> 
> > > Sort-of... I still don't see a race in what we propose but I might be
> > > missing something subtle. We are talking about two drivers for two
> > > different IP blocks updating different counters etc...  
> > 
> > If you do only *that* you should be fine. That should technically be ok.  
> 
> Yes, that' the point.

I don't think the EDAC core has any troubles with concurrency. 

As far as I remember, the hacks we had to do at x86 world are due to 
concurrency at the hardware side: having two RAS codes (either between
a driver and BIOS APEI/GHES or between two drivers) accessing the same 
set of MCA registers doesn't work at the Intel chipsets I'm aware of.

> 
> > I still think, though, that the sensible thing to do is have one
> > platform driver which concentrates all RAS functionality.   
>

> I tend to disagree here. We've been down that rabbit hole in the past
> and we (Linux in general) are trying to move away from that sort of
> "platform" overarching driver as much as possible.

Without entering on ARM-specific architecture design, I tend to agree
with the principle that different hardware components would use different
RAS drivers, provided that they use the already existing RAS cores.

Up to some extend, we have already multiple RAS drivers right now.

Besides EDAC/MCE, there are for example, PCIe errors that can come via 
AER. Network drivers also report errors using different mechanisms.
I know a person that it is interested on working on an implementation
to collect disk errors using a trace-based error report mechanism.

So, at the end of the day, different errors may come from different
drivers using different non-overlapping error mechanisms.

That's said, from the admin PoV, it makes sense to have a single
daemon that collect errors from all error sources and take the
needed actions.

> 
> > It is the
> > more sensible design and takes care of potential EDAC shortcomings and
> > the need to communicate between the different logging functionality,
> > as in, for example, "I had so many errors, lemme go and increase DRAM
> > scrubber frequency." For example. And all the other advantages of having
> > everything in a single driver.  
> 
> This is a policy. It should either belong to userspace, or be in some
> generic RAS code in the kernel, there's no reason why these can't be
> abstracted. Also in your specific example, it could be entirely local
> to the MC EDAC / DRAM controller path, we could have a generic way for
> EDAC to advertise that a given memory channel is giving lots of errors
> and have memory controller drivers listen to it but usually the EDAC MC
> driver *is* the only thing that looks like a MC driver to begin with,
> so again, pretty much no overlap with L1/L2 caches RAS or PCIe RAS
> etc...

On userspace, I guess there's just the rasdaemon and the old legacy
edac-utils. 

Right now, the rasdaemon doesn't have anything like that, but we keep
integrating new RAS report mechanisms to it (we just added support
this week for network devlink error reports). 

If I had to put a policy like that on userspace, rasdaemon should probably 
be the right place to add it.

Thanks,
Mauro