linux-kernel - Re: [PATCH v4 6/7] btrfs: rename brtfs_io_stripe::is_scrub to commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f184b3ac-7bf7-45cb-b126-3da7c52980ad@gmx.com>
Date: Sat, 6 Jul 2024 09:02:08 +0930
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Johannes Thumshirn <jth@...nel.org>, Chris Mason <clm@...com>,
 Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>
Cc: linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
 Johannes Thumshirn <johannes.thumshirn@....com>
Subject: Re: [PATCH v4 6/7] btrfs: rename brtfs_io_stripe::is_scrub to
 commit_root



在 2024/7/6 00:43, Johannes Thumshirn 写道:
> From: Johannes Thumshirn <johannes.thumshirn@....com>
>
> Rename brtfs_io_stripe's is_scrub to commit_root, as this is what it
> actually does, instruct btrfs_get_raid_extent_offset() to look at the
> commit root.

The commit_root name looks a little confusing to me.

Yes, it is to indicate whether we should search commit root, but since
only scrub (and dev-replace) is doing such behavior, it doesn't looks
that odd.

Furthermore, commit_root is way more common in btrfs_root::commit_root,
the same name can lead to different meaning
(btrfs_io_stripe::commit_root means whether to search commit root, while
btrfs_root::commit_root means commit root node).

I'd prefer something like "search_commit_root" if we're really going to
do a rename.

Thanks,
Qu
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@....com>
> ---
>   fs/btrfs/bio.c              | 2 +-
>   fs/btrfs/raid-stripe-tree.c | 2 +-
>   fs/btrfs/scrub.c            | 2 +-
>   fs/btrfs/volumes.h          | 2 +-
>   4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index f04d93109960..5f36c75a2457 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -679,7 +679,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
>   	blk_status_t ret;
>   	int error;
>
> -	smap.is_scrub = !bbio->inode;
> +	smap.commit_root = !bbio->inode;
>
>   	btrfs_bio_counter_inc_blocked(fs_info);
>   	error = btrfs_map_block(fs_info, btrfs_op(bio), logical, &map_length,
> diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c
> index ba0733c6be76..39085ff971c9 100644
> --- a/fs/btrfs/raid-stripe-tree.c
> +++ b/fs/btrfs/raid-stripe-tree.c
> @@ -259,7 +259,7 @@ int btrfs_get_raid_extent_offset(struct btrfs_fs_info *fs_info,
>   	if (!path)
>   		return -ENOMEM;
>
> -	if (stripe->is_scrub) {
> +	if (stripe->commit_root) {
>   		path->skip_locking = 1;
>   		path->search_commit_root = 1;
>   	}
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 14a8d7100018..9c483b799cf1 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1688,7 +1688,7 @@ static void scrub_submit_extent_sector_read(struct scrub_ctx *sctx,
>   					    (i << fs_info->sectorsize_bits);
>   			int err;
>
> -			io_stripe.is_scrub = true;
> +			io_stripe.commit_root = true;
>   			stripe_len = (nr_sectors - i) << fs_info->sectorsize_bits;
>   			/*
>   			 * For RST cases, we need to manually split the bbio to
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 37a09ebb34dd..25bc68af0df8 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -444,7 +444,7 @@ struct btrfs_io_stripe {
>   	/* Block mapping. */
>   	u64 physical;
>   	u64 length;
> -	bool is_scrub;
> +	bool commit_root;
>   	/* For the endio handler. */
>   	struct btrfs_io_context *bioc;
>   };
>