Re: [PATCH] btrfs: sysfs: export dev stats in devinfo directory

From: Anand Jain <anand.jain@oracle.com>
To: dsterba@suse.cz, David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: sysfs: export dev stats in devinfo directory
Date: Wed, 9 Jun 2021 15:43:44 +0800	[thread overview]
Message-ID: <8e255fb8-9dc2-57cf-f6e8-d1c23aa43563@oracle.com> (raw)
In-Reply-To: <20210607185556.GL31483@twin.jikos.cz>


On 8/6/21 2:55 am, David Sterba wrote:
> On Sat, Jun 05, 2021 at 06:38:16AM +0800, Anand Jain wrote:
>> On 04/06/2021 22:21, David Sterba wrote:
>>> On Fri, Jun 04, 2021 at 09:41:09PM +0800, Anand Jain wrote:
>>>> On 4/6/21 9:20 pm, David Sterba wrote:
>>>>> The device stats can be read by ioctl, wrapped by command 'btrfs device
>>>>> stats'. Provide another source where to read the information in
>>>>> /sys/fs/btrfs/FSID/devinfo/DEVID/stats .
>>>>
>>>>     The planned stat here is errors stat.
>>>>     So why not rename this to error_stats?
>>>
>>> I think it's commonly called device stats, dev stats, so when it's in
>>> 'devinfo' it's like it's the 'stats' for the device.
>>
>>
>>> We don't have other
>>> stats, like regarding io but in that case it would make sense to
>>> distnguish the names.
>>
>> My read_policy work (which I suppose is next on your list for review)
> 
> Yeah, it's among the next things to merge once the current features
> stabilize enough.
> 
>> made sense that publishing the io-stat information locally from btrfs is
>> a good idea. So that it provides clarity if the IO is skewed to a device
>> or balanced. Which is even more essential in the case of mixed device
>> types. For now IMHO,  /sys/fs/btrfs/FSID/devinfo/DEVID/error_stats
>> is harmless.
> 
> Agreed, I thought about the same, gathering some regular io stats, so
> the error_stats makes sense.  > There's still one open question whether to
> do it all in one file or in a subdirectory error_stats/ . > The sysfs way
> is one value per file but for the stats


> I'm more inclined to follow what
> /proc/ stats do. It's more convenient to monitor stats in one file read
> than having to do 'cat error_stats/*' or with filenames as 'grep ^
> error_stats/*'.

  Agreed. I prefer one file from the convenience pov. Also, block dev has
  one file, the reason to represent it invariably at a given time [1]
    [1] https://www.kernel.org/doc/Documentation/block/stat.txt
  IMO the same applies to btrfs too.

Thanks, Anand