On Thu, Aug 12, 2021 at 04:58:30PM +0800, Anand Jain wrote:

On 11/08/2021 15:15, Nikolay Borisov wrote:

quoted

On 10.08.21 г. 18:48, Anand Jain wrote:

quoted

btrfs_chunk_readonly() checks if the given chunk is writeable. It returns
1 for readonly, and 0 for writeable. So the return argument type bool
shall suffice instead of the current type int.

Also, rename btrfs_chunk_readonly() to btrfs_chunk_writeable() as we check
if the bg is writeable, and helps to keep the logic at the parent function
simpler.

quoted

I don't see how the logic is kept simpler, given that you just invert
it.

  IMO it is simpler to read. No? In btrfs_chunk_readonly(), we consider a
  chunk is readonly when the device it is on has _no_ DEV_STATE_WRITEABLE
  flag set.  So rename to btrfs_chunk_writeable() is correct. We also use
  readonly to the filesystem.

The logic in the function is to check for each stripe if it has
writeable flag and short cuircit if not, and this follows as we're
indeed checking for the writeable status. Further there's a check for
the missing devices that can drop previous witebale status. This all
reads as the the main point is 'writeable' status, so I'm fine with the
rename.