On Thu, May 5, 2016 at 10:04 AM, Jens Axboe <axboe@xxxxxx> wrote: > On 05/05/2016 09:59 AM, Michael Callahan wrote: >> >> On Thu, May 5, 2016 at 9:27 AM, Jeff Moyer <jmoyer@xxxxxxxxxx> wrote: >>> >>> Michael Callahan <michaelcallahan@xxxxxx> writes: >>> >>>> Separate out discards from writes counts in /block/diskstats as well as >>>> the disk related /sys stat files. >>>> >>>> Sign-off-by: Michael Callahan <michaelcallahan@xxxxxx> >>>> --- >>>> The STAT_* macros might be better defined somewhere else. >>>> >>>> The part_*_in_flight macros should probably be per_cpu. I mixed up a >>>> patch to test that but they are used awkwardly in drivers/md/dm.c and >>>> accumulating inflights on cpus would be susceptible to overflow errors >>>> on 32 bit machines (increment on one cpu, complete on another, they sum >>>> correctly until one overflows) >>>> >>>> This patch adds the new stats to the end of stat and diskstat for more >>>> backwards compatibility. However this patch breaks iostat which will >>>> need to be updated to grab the additional fields. iostat already has >>>> cases for 4 and 11 entries in stat so adding another for 15 should be >>>> easy enough and it's likely that adding these to the end of the file >>>> rather than right after the write stats is unnecessary. >>> >>> >>> You can't just break iostat. Split the fields out into another file. >>> >>> Cheers, >>> Jeff >> >> >> Would it be better to make a new stat2 file, make discard_stats, or to >> patch iostat.c >> to handle the new format? It appears that iostat.c already checks for 4 >> or 11 >> stat fields and should be easy to extend to check for 15. > > > If the new format doesn't break iostat (and others), then we don't need a > new file. If it does, we obviously do. We don't tie kernel and app upgrades > together, so asking users to upgrade iostat because it breaks with a new > kernel is not an option. > > Don't add a specific discard stats. > > -- > Jens Axboe > Tested iostat more and it appears to work fine with this patch. Looking at the code iostat.c pulls the first 11 fields and ignores the new ones. What else needs to be checked? -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
