Re: [PATCH 1/3] x86/MCE, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers

[Borislav Petkov <bp@alien8.de>]

On Tue, Oct 25, 2022 at 04:35:15PM +0000, Yazen Ghannam wrote:
> I think the "right way" to use tracepoints is to parse the data according to
> the format provided by the tracepoint itself. You can see an example of
> rasdaemon doing that here:
> https://github.com/mchehab/rasdaemon/blob/c2255178a49f62c53009a456bc37dd5e37332f09/ras-mce-handler.c#L386

Lemme add Rostedt.

So now we have libtraceevent and here's an example how to do it:

https://patchwork.kernel.org/project/linux-trace-devel/patch/20221021182345.092cdb50@gandalf.local.home/
https://www.trace-cmd.org/Documentation/libtracefs/libtracefs-kprobes.html

Reportedly, rasdaemon is still using the old libtraceevent method. So it
probably should be updated to use the new library version.

> A tracepoint user should not assume a fixed struct layout, so adding
> and rearranging fields shouldn't be a problem. I'm not sure about
> removing a field. It seems to me that this should be okay in the
> interface, and it's up to the application how it wants to handle a
> field that isn't found.

From looking at the examples, I think the new libtraceevent should be
able to handle all that just fine.

> Another option could be to define a new tracepoint.

Yeah, no. Let's get this one to work pls.

trace_mce_record2() looks like the old attempts to design a syscall
better. :)

>  Userspace already needs to be updated to recognize new fields, so I
> don't think it's much of a stretch to add a new tracepoint handler.

Syncing with other components is always a stretch. You're forgetting how
distros lag behind and don't always have the needed resources to update
their userspace.

Or they can't update because of enterprise raisins.

So no, it is not trivial, trust me. It's a bunch of politics and effort.

> This may be simpler than trying to fit vendor-specific info into an
> existing tracepoint and then decoding it later in userspace.

Until that new tracepoint becomes insufficient in the future and we need
to add trace_mce_record3(). No, you don't want that. :)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette