On Thu, Jun 10, 2021 at 06:37:01PM +0200, David Sterba wrote:

On Wed, Jun 09, 2021 at 05:55:05PM -0700, Omar Sandoval wrote:

quoted

quoted

quoted

The ioctl returns ENODEV is !dev_stats_valid, maybe this file should do
the same? It seems a little awkward to have a flag that means that the
rest of the file is meaningless.

You mean returning -ENODEV when reading the stats file? Or return 0 but
the contents is something like 'stats invalid' or similar.

I'd vote for returning -ENODEV when reading the stats file, but I think
either one is fine.

Hm so I think this should reflect how the sysfs files are used. They all
contain textual information, and errors are returned when eg. there are
no permissions.

In a shell script it's IMHO more convenient to do

stats=$(cat $devicepath/stats)

and then validate contents of $stats rather then catching the error
value and deciding based on that what happend. Not to say that this
would also print an error message. I've found this in
admin-guide/sysfs-rules.rst that's perhaps closest to a recommendation
we could follow:

172 - When reading and writing sysfs device attribute files, avoid dependency
173     on specific error codes wherever possible. This minimizes coupling to
174     the error handling implementation within the kernel.

So I take it as that error codes belong to the sysfs layer and the
validity of the contents is up to the sysfs user, ie. btrfs in this
case.

Good point. "Real" programs will just use the ioctl, so it makes sense
to cater this to shell scripts.