Re: [PATCH] btrfs: reduce return argument of btrfs_chunk_readonly

[David Sterba <dsterba@suse.cz>]

On Thu, Aug 12, 2021 at 04:58:30PM +0800, Anand Jain wrote:
> On 11/08/2021 15:15, Nikolay Borisov wrote:
> > On 10.08.21 г. 18:48, Anand Jain wrote:
> >> 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.
> 
> > 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.