Re: [PATCH] btrfs: Turn fs_roots_radix in btrfs_fs_info into an XArray

[Gabriel Niebler <gniebler@suse.com>]

Am 26.04.22 um 23:45 schrieb Gabriel Niebler:
> … rename it to simply fs_roots and adjust all usages of this object to use
> the XArray API, because it is notionally easier to use and unserstand, as
> it provides array semantics, and also takes care of locking for us,
> further simplifying the code.
> 
> Also do some refactoring, esp. where the API change requires largely
> rewriting some functions, anyway.
> 
> Signed-off-by: Gabriel Niebler <gniebler@suse.com>
> ---
>   fs/btrfs/ctree.h       |   5 +-
>   fs/btrfs/disk-io.c     | 176 ++++++++++++++++++++---------------------
>   fs/btrfs/inode.c       |  13 +--
>   fs/btrfs/transaction.c |  67 +++++++---------
>   4 files changed, 126 insertions(+), 135 deletions(-)
> 
> [...]
> @@ -2346,28 +2340,23 @@ void btrfs_put_root(struct btrfs_root *root)
>   
>   void btrfs_free_fs_roots(struct btrfs_fs_info *fs_info)
>   {
> -	int ret;
> -	struct btrfs_root *gang[8];
> -	int i;
> +	struct btrfs_root *root;
> +	unsigned long index = 0;
>   
>   	while (!list_empty(&fs_info->dead_roots)) {
> [...]   
> -		if (test_bit(BTRFS_ROOT_IN_RADIX, &gang[0]->state))
> -			btrfs_drop_and_free_fs_root(fs_info, gang[0]);
> -		btrfs_put_root(gang[0]);
> +		if (test_bit(BTRFS_ROOT_IN_RADIX, &root->state))
> +			btrfs_drop_and_free_fs_root(fs_info, root);
> +		btrfs_put_root(root);

It occurs to me that BTRFS_ROOT_IN_RADIX should probably be renamed to 
something else within or following this patch...

... but what to rename it /to/?

Naming things is hard. Here are some ideas I've had:

BTRFS_ROOT_IN_XARRAY is obvious, but it also includes kind of a needless 
implementation detail.

BTRFS_ROOT_IN_(FS_)ROOTS would be technically accurate, but might be 
confusing for the reader (e.g. if they don't know about 
btrfs_fs_indo->fs_roots of the top off their head).

BTRFS_ROOT_IN_FS_INFO_ROOTS makes that a bit clearer (someone might 
think it refers to fbtrfs_fs_info->roots, but when they find that 
doesn't exist they'd quickly catch on, I think), but a bit lengthy.

BTRFS_ROOT_IN_FS_INFO_FS_ROOTS is probably the most accurate, but really 
quite long...

Does anyone else have any ideas?