linux-kernel - Re: [PATCH] btrfs: remove extent buffer's redundant `len` member field

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: <ceaa0748-4190-4f0f-aca9-688ae1c7bfc3@gmx.com>
Date: Wed, 30 Apr 2025 08:04:39 +0930
From: Qu Wenruo <quwenruo.btrfs@....com>
To: Daniel Vacek <neelx@...e.com>, 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
Subject: Re: [PATCH] btrfs: remove extent buffer's redundant `len` member
 field



在 2025/4/30 00:47, Daniel Vacek 写道:
> Even super block nowadays uses nodesize for eb->len. This is since commits
> 
> 551561c34663 ("btrfs: don't pass nodesize to __alloc_extent_buffer()")
> da17066c4047 ("btrfs: pull node/sector/stripe sizes out of root and into fs_info")
> ce3e69847e3e ("btrfs: sink parameter len to alloc_extent_buffer")
> a83fffb75d09 ("btrfs: sink blocksize parameter to btrfs_find_create_tree_block")
> 
> With these the eb->len is not really useful anymore. Let's use the nodesize
> directly where applicable.

The idea looks great to me, and all the call sites look good too.

> 
> Signed-off-by: Daniel Vacek <neelx@...e.com>
> ---
> [RFC]
>   * Shall the eb_len() helper better be called eb_nodesize()? Or even rather
>     opencoded and not used at all?

However the name eb_len() is a little too generic.

Nodesize is a little easier to understand.

> 
>   fs/btrfs/accessors.c             |  4 +--
>   fs/btrfs/disk-io.c               | 11 ++++---
>   fs/btrfs/extent-tree.c           | 28 +++++++++--------
>   fs/btrfs/extent_io.c             | 54 ++++++++++++++------------------
>   fs/btrfs/extent_io.h             | 11 +++++--
>   fs/btrfs/ioctl.c                 |  2 +-
>   fs/btrfs/relocation.c            |  2 +-
>   fs/btrfs/subpage.c               |  8 ++---
>   fs/btrfs/tests/extent-io-tests.c | 12 +++----
>   fs/btrfs/zoned.c                 |  2 +-
>   10 files changed, 67 insertions(+), 67 deletions(-)
> 
> diff --git a/fs/btrfs/accessors.c b/fs/btrfs/accessors.c
> index e3716516ca387..a2bdbc7990906 100644
> --- a/fs/btrfs/accessors.c
> +++ b/fs/btrfs/accessors.c
> @@ -14,10 +14,10 @@ static bool check_setget_bounds(const struct extent_buffer *eb,
>   {
>   	const unsigned long member_offset = (unsigned long)ptr + off;
>   
> -	if (unlikely(member_offset + size > eb->len)) {
> +	if (unlikely(member_offset + size > eb_len(eb))) {
>   		btrfs_warn(eb->fs_info,
>   		"bad eb member %s: ptr 0x%lx start %llu member offset %lu size %d",
> -			(member_offset > eb->len ? "start" : "end"),
> +			(member_offset > eb_len(eb) ? "start" : "end"),
>   			(unsigned long)ptr, eb->start, member_offset, size);
>   		return false;
>   	}
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 3592300ae3e2e..31eb7419fe11f 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -190,7 +190,7 @@ static int btrfs_repair_eb_io_failure(const struct extent_buffer *eb,
>   	for (int i = 0; i < num_extent_folios(eb); i++) {
>   		struct folio *folio = eb->folios[i];
>   		u64 start = max_t(u64, eb->start, folio_pos(folio));
> -		u64 end = min_t(u64, eb->start + eb->len,
> +		u64 end = min_t(u64, eb->start + fs_info->nodesize,
>   				folio_pos(folio) + eb->folio_size);
>   		u32 len = end - start;
>   		phys_addr_t paddr = PFN_PHYS(folio_pfn(folio)) +
> @@ -230,7 +230,7 @@ int btrfs_read_extent_buffer(struct extent_buffer *eb,
>   			break;
>   
>   		num_copies = btrfs_num_copies(fs_info,
> -					      eb->start, eb->len);
> +					      eb->start, fs_info->nodesize);
>   		if (num_copies == 1)
>   			break;
>   
> @@ -260,6 +260,7 @@ blk_status_t btree_csum_one_bio(struct btrfs_bio *bbio)
>   {
>   	struct extent_buffer *eb = bbio->private;
>   	struct btrfs_fs_info *fs_info = eb->fs_info;
> +	u32 nodesize = fs_info->nodesize;

Minor nitpick, @nodesize can be made const.

Otherwise looks good to me.

Thanks,
Qu