FAILED: patch "[PATCH] btrfs: raid56: do not verify the content if there is no data checksum" failed to apply to 7.3-stable tree
Subsystems (2)
Sasha Levin
maintainerThe patch below does not apply to the 7.3-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-7.3.y
git checkout FETCH_HEAD
git cherry-pick -x d00b85308abee300dfb7e1ac642f38bab9ec7492
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '[email protected]' --subject-prefix 'PATCH 7.3.y' 'HEAD^..'
Thanks,
Sasha
------------------ original commit in Linus's tree ------------------
From d00b85308abee300dfb7e1ac642f38bab9ec7492 Mon Sep 17 00:00:00 2001
From: Qu Wenruo <[email protected]>
Date: Wed, 30 Sep 2026 18:29:58 +0930
Subject: [PATCH] btrfs: raid56: do not verify the content if there is no data
checksum
[BUG]
There is a bug report that fio can fail to sync the data on a degraded RAID56 mount:
# mkfs.btrfs -m raid1 -d raid5 -f $dev1 $dev2 $dev3 # mount $dev1 $mnt # btrfs balance start -d $mnt # umount $mnt # btrfs dev scan -u $dev2 # wipefs -fa $dev2 # mount -o degraded $dev1 $mnt # fio --name=degraded-write --directory=$mnt --rw=randwrite --fdatasync=4 \ --bs=4k --size=512M --randrepeat=1
The run will fail at fsync:
fio: io_u error on file /mnt/btrfs/degraded-write.0.0: Input/output error: datasync offset=142254080, buflen=0
fio: pid=2064, err=5/file:io_u.c:2008, func=io_u error, error=Input/output error
[CAUSE]
With extra debug output, it shows the problem pretty well:
rbio=2876243968 faila=0 sectornr=0
rbio=2876243968 error=0-15 csum=2 bio=15
The first line shows the rbio's full stripe logical, and stripe/sector number when verify_one_sector() failed.
The second line shows more detail about the rbio, the sector range [0, 16) is corrupted, which indicates the first data stripe is corrupted, matching the missing device.
The important part is that the csum bitmap shows the data csum is only for sector number 2, meanwhile the failed verification is at sector number 0.
This means we did the checksum verification even if no corresponding checksum is populated.
[FIX]
There is already a similar check at verify_bio_data_sectors(), but unfortunately there is no such check at verify_one_sector().
Add the missing check to skip the csum verification if there is no corresponding data checksum.
--- fs/btrfs/raid56.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/btrfs/raid56.c +8 -3
@@ -1889,6 +1889,8 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio, {struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;phys_addr_t *paddrs; +const unsigned int total_sector_nr = + stripe_nr * rbio->stripe_nsectors + sector_nr;u8 csum_buf[BTRFS_CSUM_SIZE ];u8 *csum_expected;
@@ -1898,6 +1900,11 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio,/* No way to verify P/Q as they are not covered by data csum. */ if (stripe_nr >= rbio->nr_data)return 0 ; + +/* No data checksum for this sector. */ +if (!test_bit (total_sector_nr, rbio->csum_bitmap)) +return 0 ; +/* * If we're rebuilding a read, we have to use pages from the * bio list if possible.
@@ -1908,9 +1915,7 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio, paddrs =rbio_stripe_paddrs (rbio, stripe_nr, sector_nr); } - csum_expected = rbio->csum_buf + - (stripe_nr * rbio->stripe_nsectors + sector_nr) * - fs_info->csum_size; + csum_expected = rbio->csum_buf + total_sector_nr * fs_info->csum_size;calculate_block_csum_paddrs (fs_info, paddrs, csum_buf);if (unlikely (memcmp (csum_buf, csum_expected, fs_info->csum_size) !=0 ))return -EIO ;
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 1/5 btrfs: replace btrfs_repair_io_failure() to use bio for page iteration
From: Qu Wenruo <[email protected]>
[ Upstream commit 15ed4a938b30fcb72e96187f36e2c4ff0e6e2227 ]
Currently btrfs_repair_io_failure() uses a @paddrs[] array to iterate pages.
Such a parameter is required for bs > ps cases, as one fs block crosses several pages.
However there is a much simpler and existing way to iterate pages: bio and bvec_iter.
This changes btrfs_repair_io_failure() by:
- Use a const @bvec_iter pointer to locate where the pages are
- Extract file offset/logical from the @bbio
- Require no @step parameter
Above features allow us to shorten the parameter list.
- Rename the function to btrfs_repair_bbio_failure()
- Change the caller in btrfs_repair_eb_io_failure() to allocate a bbio Unlike the data read path, we do not have a handy bbio in that case. So we need to allocate one just for btrfs_repair_bbio_failure().
- Change the error reporting in btrfs_repair_bbio_failure() to include root id and use inode number directly
Now for btree inode we will report a proper inode number (1).
Stable-dep-of: d00b85308abe ("btrfs: raid56: do not verify the content if there is no data checksum")
--- fs/btrfs/bio.c | 61 ++++++++++++++++++++++++++-------------------- fs/btrfs/bio.h | 5 ++-- fs/btrfs/disk-io.c | 25 +++++++++++++------ 3 files changed, 55 insertions(+), 36 deletions(-)
fs/btrfs/bio.c +35 -26
@@ -186,7 +186,6 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,*/ struct bvec_iter saved_iter = repair_bbio->saved_iter;const u32 step =min (fs_info->sectorsize,PAGE_SIZE ); -const u64 logical = repair_bbio->saved_iter.bi_sector <<SECTOR_SHIFT ;const u32 nr_steps = repair_bbio->saved_iter.bi_size / step;int mirror = repair_bbio->mirror_num;phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE /PAGE_SIZE ];
@@ -220,9 +219,8 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,do { mirror =prev_repair_mirror (fbio, mirror); -btrfs_repair_io_failure (fs_info,btrfs_ino (inode), - repair_bbio->file_offset, fs_info->sectorsize, - logical, paddrs, step, mirror); +btrfs_repair_bbio_failure (repair_bbio, &repair_bbio->saved_iter, + fs_info->sectorsize, mirror); }while (mirror != fbio->bbio->mirror_num); done:
@@ -925,21 +923,23 @@ void btrfs_submit_bbio(struct btrfs_bio *bbio, int mirror_num)* The I/O is issued synchronously to block the repair read completion from * freeing the bio. * -* @ino: Offending inode number -* @fileoff: File offset inside the inode +* @bbio: Original bbio where the repair is needed +* @orig_iter: Points to where the repair start is * @length: Length of the repair write -* @logical: Logical address of the range -* @paddrs: Physical address array of the content -* @step: Length of for each paddrs * @mirror_num: Mirror number to write to. Must not be zero */ -int btrfs_repair_io_failure (struct btrfs_fs_info *fs_info,u64 ino,u64 fileoff, -u32 length,u64 logical,const phys_addr_t paddrs[], -unsigned int step,int mirror_num) +int btrfs_repair_bbio_failure (struct btrfs_bio *bbio,const struct bvec_iter *orig_iter, +u32 length,int mirror_num) { -const u32 nr_steps =DIV_ROUND_UP_POW2 (length, step); +struct btrfs_inode *inode = bbio->inode; +struct btrfs_fs_info *fs_info = inode->root->fs_info;struct btrfs_io_stripe smap = {0 }; -struct bio *bio =NULL ; +struct bvec_iter iter = *orig_iter; +struct bio *repair_bio =NULL ; +const u64 logical = iter.bi_sector <<SECTOR_SHIFT ; +const u64 fileoff = bbio->file_offset + + ((iter.bi_sector - bbio->saved_iter.bi_sector) <<SECTOR_SHIFT ); +u32 cur =0 ;int ret =0 ;BUG_ON (!mirror_num);
@@ -950,8 +950,9 @@ int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff,ASSERT (IS_ALIGNED (fileoff, fs_info->sectorsize));/* Either it's a single data or metadata block. */ ASSERT (length <=BTRFS_MAX_BLOCKSIZE ); -ASSERT (step <= length); -ASSERT (is_power_of_2 (step)); + +/* Our current iter should not be before the original bbio saved_iter. */ +ASSERT (iter.bi_sector >= bbio->saved_iter.bi_sector);/* * The fs either mounted RO or hit critical errors, no need
@@ -979,15 +980,22 @@ int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff, goto out_counter_dec; } - bio =bio_alloc (smap.dev->bdev, nr_steps,REQ_OP_WRITE |REQ_SYNC ,GFP_NOFS ); - bio->bi_iter.bi_sector = smap.physical >>SECTOR_SHIFT ; -for (int i =0 ; i < nr_steps; i++) { - ret =bio_add_page (bio,phys_to_page (paddrs[i]), step,offset_in_page (paddrs[i])); -/* We should have allocated enough slots to contain all the different pages. */ -ASSERT (ret == step); + repair_bio =bio_alloc (smap.dev->bdev,max (1 , length >>PAGE_SHIFT ), +REQ_OP_WRITE |REQ_SYNC ,GFP_NOFS ); + repair_bio->bi_iter.bi_sector = smap.physical >>SECTOR_SHIFT ; +while (cur < length) { +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), length - cur); + + ret =bio_add_page (repair_bio, page, cur_len, pg_off); +ASSERT (ret == cur_len); +bio_advance_iter_single (&bbio->bio, &iter, cur_len); + cur += cur_len; } - ret =submit_bio_wait (bio); -bio_put (bio); + + ret =submit_bio_wait (repair_bio); +bio_put (repair_bio);if (ret) {/* try to remap that extent elsewhere? */ btrfs_dev_stat_inc_and_print (smap.dev,BTRFS_DEV_STAT_WRITE_ERRS );
@@ -995,8 +1003,9 @@ int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff, }btrfs_info_rl (fs_info, -"read error corrected: ino %llu off %llu (dev %s sector %llu)" , - ino, fileoff,btrfs_dev_name (smap.dev), +"read error corrected: root %llu ino %llu off %llu (dev %s sector %llu)" , +btrfs_root_id (inode->root),btrfs_ino (inode), fileoff, +btrfs_dev_name (smap.dev), smap.physical >>SECTOR_SHIFT ); ret =0 ;
fs/btrfs/bio.h +2 -3
@@ -126,8 +126,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status);void btrfs_submit_bbio (struct btrfs_bio *bbio,int mirror_num);void btrfs_submit_repair_write (struct btrfs_bio *bbio,int mirror_num,bool dev_replace); -int btrfs_repair_io_failure (struct btrfs_fs_info *fs_info,u64 ino,u64 fileoff, -u32 length,u64 logical,const phys_addr_t paddrs[], -unsigned int step,int mirror_num); +int btrfs_repair_bbio_failure (struct btrfs_bio *bbio,const struct bvec_iter *orig_iter, +u32 length,int mirror_num);#endif
fs/btrfs/disk-io.c +18 -7
@@ -176,19 +176,24 @@ static int btrfs_repair_eb_io_failure(const struct extent_buffer *eb, int mirror_num) {struct btrfs_fs_info *fs_info = eb->fs_info; -const u32 step =min (fs_info->nodesize,PAGE_SIZE ); -const u32 nr_steps = eb->len / step; -phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE /PAGE_SIZE ]; +struct btrfs_bio *bbio; +int ret;if (sb_rdonly (fs_info->sb))return -EROFS ; +/* +* This bbio is only to queue all pages for btrfs_repair_bbio_failure(). +* Thus it will never get its endio called. +*/ + bbio =btrfs_bio_alloc (max (1 , fs_info->nodesize >>PAGE_SHIFT ),REQ_OP_READ , +BTRFS_I (fs_info->btree_inode), eb->start,NULL ,NULL ); + bbio->bio.bi_iter.bi_sector = eb->start >>SECTOR_SHIFT ;for (int i =0 ; i <num_extent_pages (eb); i++) {struct folio *folio = eb->folios[i];/* No large folio support yet. */ ASSERT (folio_order (folio) ==0 ); -ASSERT (i < nr_steps);/* * For nodesize < page size, there is just one paddr, with some
@@ -197,11 +202,17 @@ static int btrfs_repair_eb_io_failure(const struct extent_buffer *eb,* For nodesize >= page size, it's one or more paddrs, and eb->start * must be aligned to page boundary. */ - paddrs[i] =page_to_phys (&folio->page) +offset_in_page (eb->start); + ret =bio_add_page (&bbio->bio, &folio->page,min (PAGE_SIZE , fs_info->nodesize), +offset_in_page (eb->start)); +ASSERT (ret ==min (PAGE_SIZE , fs_info->nodesize)); } +/* Since the bbio is never submitted, we have to save the iter manually. */ + bbio->saved_iter = bbio->bio.bi_iter; -return btrfs_repair_io_failure (fs_info,0 , eb->start, eb->len, - eb->start, paddrs, step, mirror_num); + ret =btrfs_repair_bbio_failure (bbio, &bbio->saved_iter, fs_info->nodesize, + mirror_num); +bio_put (&bbio->bio); +return ret; }/*
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/5 btrfs: enhance btrfs_data_csum_ok() to use bio for page iteration
From: Qu Wenruo <[email protected]>
[ Upstream commit 3e4a083bf0c8bb727517ae781052b80167443077 ]
Currently btrfs_data_csum_ok() requires a @paddr[] array to iterate all possible pages for bs > ps cases.
However for all btrfs_data_csum_ok() call sites, we already have a btrfs_bio, and the bio infrastructure has many flexible ways to iterate multiple pages already.
Change btrfs_data_csum_ok() to make full use of btrfs_bio by:
- Change the parameter list to require a @bvec_iter pointer And remove @bio_offset, which can be calculated through @bvec_iter and bbio->saved_iter.
Also remove paddrs[], we will iterate all the pages using bio interfaces.
- Make the same parameter changes to repair_one_sector()
- Use bio interfaces to iterate pages from a bio
- Rename the function to btrfs_bio_data_csum_ok()
- Remove on-stack paddrs[] array usage
Stable-dep-of: d00b85308abe ("btrfs: raid56: do not verify the content if there is no data checksum")
--- fs/btrfs/bio.c | 82 +++++++++++++++--------------------------- fs/btrfs/btrfs_inode.h | 4 +-- fs/btrfs/inode.c | 49 +++++++++++++++++++------ 3 files changed, 70 insertions(+), 65 deletions(-)
fs/btrfs/bio.c +29 -53
@@ -180,29 +180,13 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,struct btrfs_failed_bio *fbio = repair_bbio->private;struct btrfs_inode *inode = repair_bbio->inode;struct btrfs_fs_info *fs_info = inode->root->fs_info; -/* -* We can not move forward the saved_iter, as it will be later -* utilized by repair_bbio again. -*/ -struct bvec_iter saved_iter = repair_bbio->saved_iter; -const u32 step =min (fs_info->sectorsize,PAGE_SIZE ); -const u32 nr_steps = repair_bbio->saved_iter.bi_size / step;int mirror = repair_bbio->mirror_num; -phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE /PAGE_SIZE ]; -phys_addr_t paddr; -unsigned int slot =0 ; -/* Repair bbio should be eaxctly one block sized. */ +/* Repair bbio should be exactly one block sized. */ ASSERT (repair_bbio->saved_iter.bi_size == fs_info->sectorsize); -btrfs_bio_for_each_block (paddr, &repair_bbio->bio, &saved_iter, step) { -ASSERT (slot < nr_steps); - paddrs[slot] = paddr; - slot++; - } -if (repair_bbio->bio.bi_status || - !btrfs_data_csum_ok (repair_bbio, dev,0 , paddrs)) { + !btrfs_bio_data_csum_ok (repair_bbio, &repair_bbio->saved_iter, dev)) {bio_reset (&repair_bbio->bio,NULL ,REQ_OP_READ ); repair_bbio->bio.bi_iter = repair_bbio->saved_iter;
@@ -236,25 +220,21 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,* read succeeded to restore the redundancy. */ static struct btrfs_failed_bio *repair_one_sector (struct btrfs_bio *failed_bbio, -u32 bio_offset, -phys_addr_t paddrs[], +const struct bvec_iter *orig_iter,struct btrfs_failed_bio *fbio) {struct btrfs_inode *inode = failed_bbio->inode;struct btrfs_fs_info *fs_info = inode->root->fs_info; -const u32 sectorsize = fs_info->sectorsize; -const u32 step =min (fs_info->sectorsize,PAGE_SIZE ); -const u32 nr_steps = sectorsize / step; -/* -* For bs > ps cases, the saved_iter can be partially moved forward. -* In that case we should round it down to the block boundary. -*/ -const u64 logical =round_down (failed_bbio->saved_iter.bi_sector <<SECTOR_SHIFT , - sectorsize);struct btrfs_bio *repair_bbio;struct bio *repair_bio; +struct bvec_iter iter = *orig_iter; +const u32 sectorsize = fs_info->sectorsize; +const u32 bio_offset = ((iter.bi_sector - failed_bbio->saved_iter.bi_sector) << +SECTOR_SHIFT ); +const u64 logical = (iter.bi_sector <<SECTOR_SHIFT );int num_copies;int mirror; +u32 cur =0 ;btrfs_debug (fs_info,"repair read error: read error at %llu" , failed_bbio->file_offset + bio_offset);
@@ -275,17 +255,21 @@ static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,atomic_inc (&fbio->repair_count); - repair_bio =bio_alloc_bioset (NULL , nr_steps,REQ_OP_READ ,GFP_NOFS , - &btrfs_repair_bioset); + repair_bio =bio_alloc_bioset (NULL ,max (1 , sectorsize >>PAGE_SHIFT ), +REQ_OP_READ ,GFP_NOFS , &btrfs_repair_bioset); repair_bio->bi_iter.bi_sector = logical >>SECTOR_SHIFT ; -for (int i =0 ; i < nr_steps; i++) { +while (cur < sectorsize) { +struct page *page =bio_iter_page (&failed_bbio->bio, iter); +const u32 pg_off =bio_iter_offset (&failed_bbio->bio, iter); +const u32 cur_len =min (bio_iter_len (&failed_bbio->bio, iter), + sectorsize - cur);int ret; -ASSERT (offset_in_page (paddrs[i]) + step <=PAGE_SIZE ); + ret =bio_add_page (repair_bio, page, cur_len, pg_off); +ASSERT (ret == cur_len); - ret =bio_add_page (repair_bio,phys_to_page (paddrs[i]), step, -offset_in_page (paddrs[i])); -ASSERT (ret == step); +bio_advance_iter_single (&failed_bbio->bio, &iter, cur_len); + cur += cur_len; } repair_bbio =btrfs_bio (repair_bio);
@@ -303,18 +287,16 @@ static void btrfs_check_read_bio(struct btrfs_bio *bbio, struct btrfs_device *destruct btrfs_inode *inode = bbio->inode;struct btrfs_fs_info *fs_info = inode->root->fs_info;const u32 sectorsize = fs_info->sectorsize; -const u32 step =min (sectorsize,PAGE_SIZE ); -const u32 nr_steps = sectorsize / step; -struct bvec_iter *iter = &bbio->saved_iter; +struct bvec_iter iter;blk_status_t status = bbio->bio.bi_status;struct btrfs_failed_bio *fbio =NULL ; -phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE /PAGE_SIZE ]; -phys_addr_t paddr; -u32 offset =0 ;/* Read-repair requires the inode field to be set by the submitter. */ ASSERT (inode); +/* The original bbio should be sectorsize aligned. */ +ASSERT (IS_ALIGNED (bbio->saved_iter.bi_size, sectorsize)); +/* * Hand off repair bios to the repair code as there is no upper level * submitter for them.
@@ -327,16 +309,10 @@ static void btrfs_check_read_bio(struct btrfs_bio *bbio, struct btrfs_device *de/* Clear the I/O error. A failed repair will reset it. */ bbio->bio.bi_status =BLK_STS_OK ; -btrfs_bio_for_each_block (paddr, &bbio->bio, iter, step) { - paddrs[(offset / step) % nr_steps] = paddr; - offset += step; - -if (IS_ALIGNED (offset, sectorsize)) { -if (status || - !btrfs_data_csum_ok (bbio, dev, offset - sectorsize, paddrs)) - fbio =repair_one_sector (bbio, offset - sectorsize, - paddrs, fbio); - } +for (iter = bbio->saved_iter; iter.bi_size; +bio_advance_iter (&bbio->bio, &iter, sectorsize)) { +if (status || !btrfs_bio_data_csum_ok (bbio, &iter, dev)) + fbio =repair_one_sector (bbio, &iter, fbio); }if (bbio->csum != bbio->csum_inline)kvfree (bbio->csum);
@@ -924,7 +900,7 @@ void btrfs_submit_bbio(struct btrfs_bio *bbio, int mirror_num)* freeing the bio. * * @bbio: Original bbio where the repair is needed -* @orig_iter: Points to where the repair start is +* @orig_iter: Points to where the repair starts * @length: Length of the repair write * @mirror_num: Mirror number to write to. Must not be zero */
fs/btrfs/btrfs_inode.h +2 -2
@@ -513,8 +513,8 @@ void btrfs_calculate_block_csum_pages(struct btrfs_fs_info *fs_info, const phys_addr_t paddrs[], u8 *dest);int btrfs_check_block_csum (struct btrfs_fs_info *fs_info,phys_addr_t paddr,u8 *csum,const u8 *const csum_expected); -bool btrfs_data_csum_ok (struct btrfs_bio *bbio,struct btrfs_device *dev, -u32 bio_offset,const phys_addr_t paddrs[]); +bool btrfs_bio_data_csum_ok (struct btrfs_bio *bbio,const struct bvec_iter *orig_iter, +struct btrfs_device *dev); noinline int can_nocow_extent(struct btrfs_inode *inode, u64 offset, u64 *len,struct btrfs_file_extent *file_extent,bool nowait);
fs/btrfs/inode.c +39 -10
81 lines not on this page · raw message
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/5 btrfs: use a shared helper to calculate data checksum for a bio
In reply to Sasha LevinFrom: Qu Wenruo <[email protected]>
[ Upstream commit 64978a4eced2b8f769cdfc27d68c6710e9c92627 ]
Since we are already calculating data checksum using bio interface, extract the generation part into btrfs_csum_one_bio_block(), and use that to replace the paddrs[] array based solution in csum_one_bio().
This will reduce 128 bytes on-stack memory usage for csum_one_bio().
Stable-dep-of: d00b85308abe ("btrfs: raid56: do not verify the content if there is no data checksum")
--- fs/btrfs/btrfs_inode.h | 2 ++ fs/btrfs/file-item.c | 19 +++++------------ fs/btrfs/inode.c | 46 +++++++++++++++++++++++++----------------- 3 files changed, 34 insertions(+), 33 deletions(-)
fs/btrfs/btrfs_inode.h +2 -0
@@ -515,6 +515,8 @@ int btrfs_check_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr, u8 const u8 * const csum_expected);bool btrfs_bio_data_csum_ok (struct btrfs_bio *bbio,const struct bvec_iter *orig_iter,struct btrfs_device *dev); +void btrfs_csum_one_bio_block (struct btrfs_fs_info *fs_info,struct bio *bio, +const struct bvec_iter *orig_iter,u8 *csum); noinline int can_nocow_extent(struct btrfs_inode *inode, u64 offset, u64 *len,struct btrfs_file_extent *file_extent,bool nowait);
fs/btrfs/file-item.c +5 -14
@@ -801,25 +801,16 @@ static void csum_one_bio(struct btrfs_bio *bbio, struct bvec_iter *src) {struct btrfs_inode *inode = bbio->inode;struct btrfs_fs_info *fs_info = inode->root->fs_info; -struct bio *bio = &bbio->bio;struct btrfs_ordered_sum *sums = bbio->sums; -struct bvec_iter iter = *src; -phys_addr_t paddr; +struct bvec_iter iter;const u32 blocksize = fs_info->sectorsize; -const u32 step =min (blocksize,PAGE_SIZE ); -const u32 nr_steps = blocksize / step; -phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE /PAGE_SIZE ]; -u32 offset =0 ;int index =0 ; -btrfs_bio_for_each_block (paddr, bio, &iter, step) { - paddrs[(offset / step) % nr_steps] = paddr; - offset += step; +for (iter = *src; iter.bi_size;bio_advance_iter (&bbio->bio, &iter, blocksize)) { +btrfs_csum_one_bio_block (fs_info, &bbio->bio, &iter, + sums->sums + index); -if (IS_ALIGNED (offset, blocksize)) { -btrfs_calculate_block_csum_pages (fs_info, paddrs, sums->sums + index); - index += fs_info->csum_size; - } + index += fs_info->csum_size; } }
fs/btrfs/inode.c +27 -19
70 lines not on this page · raw message
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 4/5 btrfs: remove on-stack paddrs[] array usage
In reply to Sasha LevinFrom: Qu Wenruo <[email protected]>
[ Upstream commit f41f4dcddea1b9eb41fc3a5491b2a8dd1f7c21ff ]
Since the bs > ps support, we have to handle cases where a data block is inside several discontiguous pages.
Thus we need a local paddrs[] array to assemble a data block for bs > ps cases.
However to handle all possible bs/ps combinations, we have to declare such array using the max block size vs page size, no matter the current block size and page size.
This adds 128 bytes on-stack memory usage for several call sites, and also introduced several duplicated helpers to calculate checksum for a data block:
- btrfs_calculate_block_csum_folio()
- btrfs_calculate_block_csum_pages()
- btrfs_check_block_csum()
The differences are mostly in how the data is passed. The first one accepts a contiguous paddr range. The second one accepts an array of paddrs[]. The last one is just a simple wrapper of the first one.
However the most common interface to iterate a data block is through bio, and we have already converted most callers to use the bio based interface, e.g. btrfs_bio_data_csum_ok() and btrfs_csum_one_bio_block().
Convert the remaining two call sites to address the remaining paddrs[] usage:
- btrfs_calculate_block_csum_pages() inside verify_bio_data_sectors() This can be switched to btrfs_csum_one_bio_block().
This removes the 128 bytes on-stack memory usage.
- btrfs_calculate_block_csum_pages() inside verify_one_sector() This call site doesn't use on-stack memory for paddrs[], but reuses the existing btrfs_raid_bio::bio_paddrs[] or btrfs_raid_bio::stripe_paddrs[].
So implement a local version called calculate_block_csum_paddrs().
Now there is no fixed on-stack paddrs[] usage anymore.
Stable-dep-of: d00b85308abe ("btrfs: raid56: do not verify the content if there is no data checksum")
--- fs/btrfs/btrfs_inode.h | 6 ---- fs/btrfs/inode.c | 75 ------------------------------------------ fs/btrfs/raid56.c | 46 +++++++++++++++----------- 3 files changed, 27 insertions(+), 100 deletions(-)
fs/btrfs/btrfs_inode.h +0 -6
@@ -507,12 +507,6 @@ static inline void btrfs_set_inode_mapping_order(struct btrfs_inode *inode) inode->root->fs_info->block_max_order); } -void btrfs_calculate_block_csum_folio (struct btrfs_fs_info *fs_info, -const phys_addr_t paddr,u8 *dest); -void btrfs_calculate_block_csum_pages (struct btrfs_fs_info *fs_info, -const phys_addr_t paddrs[],u8 *dest); -int btrfs_check_block_csum (struct btrfs_fs_info *fs_info,phys_addr_t paddr,u8 *csum, -const u8 *const csum_expected);bool btrfs_bio_data_csum_ok (struct btrfs_bio *bbio,const struct bvec_iter *orig_iter,struct btrfs_device *dev); void btrfs_csum_one_bio_block(struct btrfs_fs_info *fs_info, struct bio *bio,
fs/btrfs/inode.c +0 -75
81 lines not on this page · raw message
fs/btrfs/raid56.c +27 -19
80 lines not on this page · raw message
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 5/5 btrfs: raid56: do not verify the content if there is no data checksum
In reply to Sasha LevinFrom: Qu Wenruo <[email protected]>
[ Upstream commit d00b85308abee300dfb7e1ac642f38bab9ec7492 ]
[BUG]
There is a bug report that fio can fail to sync the data on a degraded RAID56 mount:
# mkfs.btrfs -m raid1 -d raid5 -f $dev1 $dev2 $dev3 # mount $dev1 $mnt # btrfs balance start -d $mnt # umount $mnt # btrfs dev scan -u $dev2 # wipefs -fa $dev2 # mount -o degraded $dev1 $mnt # fio --name=degraded-write --directory=$mnt --rw=randwrite --fdatasync=4 \ --bs=4k --size=512M --randrepeat=1
The run will fail at fsync:
fio: io_u error on file /mnt/btrfs/degraded-write.0.0: Input/output error: datasync offset=142254080, buflen=0
fio: pid=2064, err=5/file:io_u.c:2008, func=io_u error, error=Input/output error
[CAUSE]
With extra debug output, it shows the problem pretty well:
rbio=2876243968 faila=0 sectornr=0
rbio=2876243968 error=0-15 csum=2 bio=15
The first line shows the rbio's full stripe logical, and stripe/sector number when verify_one_sector() failed.
The second line shows more detail about the rbio, the sector range [0, 16) is corrupted, which indicates the first data stripe is corrupted, matching the missing device.
The important part is that the csum bitmap shows the data csum is only for sector number 2, meanwhile the failed verification is at sector number 0.
This means we did the checksum verification even if no corresponding checksum is populated.
[FIX]
There is already a similar check at verify_bio_data_sectors(), but unfortunately there is no such check at verify_one_sector().
Add the missing check to skip the csum verification if there is no corresponding data checksum.
--- fs/btrfs/raid56.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/btrfs/raid56.c +8 -3
@@ -1892,6 +1892,8 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio, {struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;phys_addr_t *paddrs; +const unsigned int total_sector_nr = + stripe_nr * rbio->stripe_nsectors + sector_nr;u8 csum_buf[BTRFS_CSUM_SIZE ];u8 *csum_expected;
@@ -1901,6 +1903,11 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio,/* No way to verify P/Q as they are not covered by data csum. */ if (stripe_nr >= rbio->nr_data)return 0 ; + +/* No data checksum for this sector. */ +if (!test_bit (total_sector_nr, rbio->csum_bitmap)) +return 0 ; +/* * If we're rebuilding a read, we have to use pages from the * bio list if possible.
@@ -1911,9 +1918,7 @@ static int verify_one_sector(struct btrfs_raid_bio *rbio, paddrs =rbio_stripe_paddrs (rbio, stripe_nr, sector_nr); } - csum_expected = rbio->csum_buf + - (stripe_nr * rbio->stripe_nsectors + sector_nr) * - fs_info->csum_size; + csum_expected = rbio->csum_buf + total_sector_nr * fs_info->csum_size;calculate_block_csum_paddrs (fs_info, paddrs, csum_buf);if (unlikely (memcmp (csum_buf, csum_expected, fs_info->csum_size) !=0 ))return -EIO ;
Signature · 1 line
2.53.0