Re: [PATCH v3 RESEND 0/4] btrfs: removal of on-stack paddrs[], final part

[Boris Burkov <boris@bur.io>]

On Wed, Aug 19, 2026 at 10:36:15AM +0930, Qu Wenruo wrote:
> [CHANGELOG]
> v3 RESEND:
> - Minor commit message change
>   The resend is most for proper sashiko review.
> 
> v3:
> - Rebased to the latest for-next
>   There is a fix in RAID56, which can cause conflicts with the last
>   patch
> 
> - Remove all remaining on-stack paddrs[] usage
>   There are two last ones in RAID56, one can be converted to use bio
>   interface, the other is not using on-stack paddrs[] array.
> 
>   So we can finally remove all on-stack paddrs[] usage.
> 
> v2:
> - Fix a missing assignment for metadata repair
>   The logical should be assigned before passing it to
>   btrfs_repair_bbio_failure().
> 
> - Move the commit message of error message change to the correct patch
>   It's changed in the first patch not the last one.
> 
> Since the experimental bs > ps support, several on-stack fixed paddrs[]
> arrays are introduced, for assemble mutli-page sized fs blocks.
> 
> However that on-stack memory usage is always there for 4K page sized
> systems, no matter if the block size of the filesystem.
> 
> The idea is to use bio interface for page iterations, the core idea is
> to use a const bvec_iter as the pointer to where the block is.
> 
> Then we save a local bevc_iter, and use the local iter to check the next
> few pages until we fill a full block.
> 
> Furthermore, with the help of bvec_iter, we can remove a lot of
> parameters:
> 
> - file_offset
> - logical
> - bio_offset
>   All can be generated by using the @iter passed in and the
>   bbio->saved_iter to calculate the old @bio_offset.
> 
>   @bio_offset is the (iter.bi_sector - saved_iter.bi_sector) <<
>   SECTOR_SHIFT.
>   As when bvec_iter is advanced, its bi_sector is also increased.
> 
>   @logical is simpler, just iter.bi_sector << SECTOR_SHIFT.
> 
>   @file_offset is the bbio->file_offset + bio_offset.
> 
> This means we no longer need to use on-stack paddrs[] to csum
> generation.

This series looks really good to me overall, thanks for making this huge
API improvement.

Reviewed-by: Boris Burkov <boris@bur.io>

> 
> With bio interfaces, the iteration of an fs block is as simple as the
> following: (I tried to change the page/pg_off/cur_len into a macro just
> like btrfs_bio_for_each_block(), but failed)
> 
> 	u32 cur = 0;
> 
> 	btrfs_csum_init(&cctx, fs_info->csum_type);
> 	while (cur < blocksize) {
> 		struct page *page = bio_iter_page(&bbio->bio, iter);
> 		const u32 pg_off = bio_iter_offset(&bbio->bio, iter);
> 		const u32 cur_len = min(bio_iter_len(&bbio->bio, iter), blocksize - cur);
> 		void *kaddr;
> 
> 		kaddr = kmap_local_page(page) + pg_off;
> 		btrfs_csum_update(&cctx, kaddr, cur_len);
> 		kunmap_local(kaddr);
> 
> 		bio_advance_iter_single(&bbio->bio, &iter, cur_len);
> 		cur += cur_len;
> 	}
> 	btrfs_csum_final(&cctx, csum);
> 
> However there is still one caller left:
> 
> - RAID56
>   That will be only location left without a bio.
>   In that case we can easily craft a local helper to do csum generation
>   without using on-stack paddrs[].
> 
> Qu Wenruo (4):
>   btrfs: replace btrfs_repair_io_failure() to use bio for page iteration
>   btrfs: enhance btrfs_data_csum_ok() to use bio for page iteration
>   btrfs: use a shared helper to calculate data checksum for a bio
>   btrfs: remove on-stack paddrs[] array usage
> 
>  fs/btrfs/bio.c         | 140 +++++++++++++++++++----------------------
>  fs/btrfs/bio.h         |   5 +-
>  fs/btrfs/btrfs_inode.h |  12 ++--
>  fs/btrfs/disk-io.c     |  25 +++++---
>  fs/btrfs/file-item.c   |  20 ++----
>  fs/btrfs/inode.c       | 114 +++++++++++----------------------
>  fs/btrfs/raid56.c      |  47 ++++++++------
>  7 files changed, 159 insertions(+), 204 deletions(-)
> 
> -- 
> 2.54.0
>