Re: [PATCH v2 2/2] btrfs: Expose the BTRFS commit stats through sysfs

[Boris Burkov <boris@bur.io>]

On Wed, Jun 15, 2022 at 03:31:30PM +0200, David Sterba wrote:
> On Wed, Jun 15, 2022 at 03:47:51PM +0300, Nikolay Borisov wrote:
> > > +
> > > +	return sysfs_emit(buf,
> > > +		"commits %llu\n"
> > > +		"last_commit_dur %llu ms\n"
> > > +		"max_commit_dur %llu ms\n"
> > > +		"total_commit_dur %llu ms\n",
> > > +		fs_info->commit_stats.commit_counter,
> > > +		fs_info->commit_stats.last_commit_dur / NSEC_PER_MSEC,
> > > +		fs_info->commit_stats.max_commit_dur / NSEC_PER_MSEC,
> > > +		fs_info->commit_stats.total_commit_dur / NSEC_PER_MSEC);
> > > +}
> > > +
> > > +static ssize_t btrfs_commit_stats_store(struct kobject *kobj,
> > > +						struct kobj_attribute *a,
> > > +						const char *buf, size_t len)
> > > +{
> > 
> > Is there really value in being able to zero out the current stats?
> 
> I think it makes sense for the max commit time, one might want to track
> that for some workload and then reset. For the other it can go both
> ways, eg. a monitoring tool saves the stats completely and resets them.
> OTOH long term stats would be lost in case there's more than one
> application reading it.

As far as I can see, our options are roughly:
1. separate files per stat, only max file has clear
2. clear only max when clearing the joint file
3. clear everything in the joint file (current patch)
4. clear bitmap to control which fields to clear

1 seems the clearest, but is sort of messy in terms of spamming lots of
files. There can be a "1b" variant which is one file with
count/total/last and a second file with max (rather than one each for all
four). 2 is a bit weird, just due to asymmetry. The "multiple separate
clearers" problem Dave came up with seems  serious for 3: it means if I
want to clear max and you want to clear total, we might make each other
lose data. 4 would work around that, but is an untuitive interface.

One other reason clearing total could be good is if we are counting in
nanoseconds, overflow becomes a non-trivial risk. For this reason, I
think I vote for 1 (separate files), but 2 (only clear max in a single
file) seems like a decent compromise. 4 feels overengineered, but is
kind of a souped-up version of 2.