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

From: Michal Krakowiak <michal.krakowiak@linux.intel.com>
To: Keith Busch <kbusch@kernel.org>, Dan Williams <dan.j.williams@intel.com>
Cc: Jens Axboe <axboe@fb.com>,
	Michal Krakowiak <michal.krakowiak@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	linux-nvme@lists.infradead.org, Sagi Grimberg <sagi@grimberg.me>
Subject: Re: [PATCH] nvme: trace: parse format nvm command in detail
Date: Mon, 11 Jan 2021 21:15:52 +0100	[thread overview]
Message-ID: <20210111201552.GA90320@michal-desk> (raw)
In-Reply-To: <20210111175018.GD1458209@dhcp-10-100-145-180.wdc.com>

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@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).

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme