Re: [PATCH 4/4] EDAC: Convert AMD EDAC pieces to use RAS printk buffer

[Mauro Carvalho Chehab <mchehab@redhat.com>]

Em 05-03-2012 09:44, Borislav Petkov escreveu:
> On Mon, Mar 05, 2012 at 08:43:59AM -0300, Mauro Carvalho Chehab wrote:
>> It is still adding an amd64-specific code inside the core, as no other driver will
>> use the amd64 "ras_agent".
> 
> Whoopie, we have another example that you're not really reading
> my emails: ras_agent is _NOT_ amd64-specific but it is defined in
> <arch/x86/ras/ras.c>

No. This is an example that you're not reading my emails: no other driver needs that.
So, it is something that it is specific to the MCA amd64 drivers.

The other two MCA drivers are sb_edac and i7core_edac. I wrote both drivers, and they
don't need any helper function to store strings on a temporary buffer.

Also, the edac core is not x86-specific. So, referencing to a var there (ras_agent) 
that it is defined inside arch/x86 would break Kernel compilation on all other 
architectures.

> [..]
> 
>> If nobody objects, I'll add my changes to linux-next, as it was tested
>> on most systems. I'll remove the MCE-specific tracepoint from my code,
>> keeping the trace there for the other stuff.
> 
> As already pointed out, I object to the tracepoints you've defined for every
> single edac_mc_handle_* call:
> 
> TRACE_EVENT(mc_corrected_error,
> TRACE_EVENT(mc_uncorrected_error,
> TRACE_EVENT(mc_corrected_error_fbd,
> TRACE_EVENT(mc_uncorrected_error_fbd,
> TRACE_EVENT(mc_out_of_range,
> TRACE_EVENT(mc_corrected_error_no_info,
> TRACE_EVENT(mc_uncorrected_error_no_info,

As already pointed out, you're not reading my emails. The above were at the version 1 of
my patches, with I sent at least a month ago. Since version 2, what is proposed is to use:

TRACE_EVENT(mc_error_mce,

for MCA-based memory error events. There's also a variant for non-MCA drivers (mc_error). 

[1] http://git.kernel.org/?p=linux/kernel/git/mchehab/linux-edac.git;a=commitdiff;h=4eb2a29419c1fefd76c8dbcd308b84a4b52faf4d

I also wrote on my emails that, instead of having a tracepoint specific for memory errors,
it is possible to re-define the fields I've proposed to cover CPU location/socket label,
and that this is better than folding everything into a hard-to-parse single string message.

> The edac drivers which get their error info from MCA should use
> trace_mce_record() and the others should either use a _single_ generic
> tracepoint or define one which adheres to the underlying hardware
> reporting scheme (be it PCI-AER, or whatever).
> 

Mauro.