[PATCH] nvme: trace: parse format nvm command in detail

[Michal Krakowiak]

On Mon, Jan 11, 2021 at 09:50:18AM -0800, Keith Busch wrote:
>On Mon, Jan 11, 2021 at 09:10:34AM -0800, Dan Williams wrote:
>> On Mon, Jan 11, 2021 at 9:01 AM Keith Busch <kbusch at kernel.org> wrote:
>> >
>> > On Fri, Jan 08, 2021 at 04:12:31PM -0800, Dan Williams wrote:
>> > > Is there a more centralized place to land such a helper? trace-cmd,
>> > > perf? The nice thing about kernel parsing is I don't need to have a
>> > > separate pile of disparate tools across subsystems.
>> >
>> > Yes, I just placed it in a temporary location to see if the other nvme
>> > stakeholders are okay with the idea. If so, I can make a real patch
>> > series to remove the in-kernel decoding, and add the parsing script
>> > somewhere in tools/.
>>
>> "Somewhere in tools/" is still not quite what I'm asking for, I want
>> something nearly as automatic and seamless as the kernel ftrace pipe.
>> If I need nvme tools for nvme events and mce tools for mce events etc,
>> I'd rather just have decode in the kernel. Perhaps this is a use case
>> for a usermode helper that accompanies a new kernel tracing file?. If
>> the helper exists the events supported events are decoded if the
>> helper is missing the interface is equivalent to the raw formatted
>> events. Decoded events "just work".
>
>My only concern is that decoding omits fields, so you have to ugprade
>your kernel in order to see the missing ones when the current kernel's
>decoding doesn't "just work". If that's how people want to maintain
>them, then I won't object any longer.

Like Dan, I really appreciate having the trace parsed in the kernel. I 
understand the concerns. What about having both: detailed parsing and 
raw bytes? It does not seem to me like a big deal to append a field with 
the raw data even if detailed parsing is implemented. Let the parsing 
extends the raw data instead of replacing them. This will result in the 
detailed human-friendly output (which is nice to have) while not losing 
data in the future (in the case that a new unsupported by parser field 
appears).