Re: [RFC PATCH 02/18] blktrace: add more definitions for BLK_TC_ACT

[Jeff Moyer <jmoyer@redhat.com>]

Christoph Hellwig <hch@infradead.org> writes:

> On Tue, Apr 30, 2019 at 09:28:15PM -0700, Chaitanya Kulkarni wrote:
>> @@ -104,7 +120,12 @@ struct blk_io_trace {
>>  	__u64 time;		/* in nanoseconds */
>>  	__u64 sector;		/* disk offset */
>>  	__u32 bytes;		/* transfer length */
>> +
>> +#ifdef CONFIG_BLKTRACE_EXT
>> +	__u64 action;		/* what happened */
>> +#else
>>  	__u32 action;		/* what happened */
>> +#endif /* CONFIG_BLKTRACE_EXT */
>
> You can't use CONFIG_ symbols in UAPI headers, as userspace
> applications won't set it.  You also can't ever change the layout of an
> existing structure in UAPI headers in not backward compatible way.

Right.  The blk_io_trace->magic has the lower 8 bits reserved for a
version number which is checked by userspace.  There's no way to
negotiate a supported version between userspace and the kernel,
unfortunately.  The version number is checked for each trace event.

What you *could* do is to add another trace event with a higher version
number that includes only the extra data.  So each event would be split
into two: the original event with original content and the new event
that only contains the new fields.  That way the old userspace would
continue to work, as it would discard the trace events it doesn't
recognize.  Newer userspace could handle both types of events, and merge
them back together.

There would be a ton of warnings spewed on stderr, unfortunately, but it
would at least work.  I don't see a lot of value in the kernel config
option, no matter which way we go with this.

-Jeff