Thanks for the reply Jeff. On 5/1/19 5:56 AM, Jeff Moyer wrote: > Christoph Hellwig <[email protected]> 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 >
