Thanks for the reply Jeff.
On 5/1/19 5:56 AM, Jeff Moyer wrote:
> Christoph Hellwig <hch@xxxxxxxxxxxxx> 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.
>
As you have mentioned this approach will have a lot of stderr, I was
trying to avoid this scenario. If everyone is okay with this will make
this change and resend the series.
> -Jeff
>
