* [PATCH 1/4] btrfs: make btrfs_csum_one_bio() handle bs > ps without large folios
2025-10-01 9:50 [PATCH 0/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
@ 2025-10-01 9:50 ` Qu Wenruo
2025-10-01 9:50 ` [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases " Qu Wenruo
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2025-10-01 9:50 UTC (permalink / raw)
To: linux-btrfs
For bs > ps cases, all folios passed into btrfs_csum_one_bio() is
ensured to be backed by large folios.
But that requirement excludes features like direct IO and encoded
writes.
To support bs > ps without large folios, enhance btrfs_csum_one_bio()
by:
- Split btrfs_calculate_block_csum() into two versions
* btrfs_calculate_block_csum_folio()
For call sites that a fs block is always backed by a large folio.
This will do extra checks on the folio size, build a paddrs[] array,
and pass it into the newer btrfs_calculate_block_csum_pages()
helper.
For now btrfs_check_block_csum() is still using this version.
* btrfs_calculate_block_csum_pages()
For call sites that may hit a fs block backed by incontiguous pages.
The pages are represented by paddrs[] array, which includes the
offset inside the page.
This function will do the proper sub-block handling.
- Make btrfs_csum_one_bio() to use btrfs_calculate_block_csum_pages()
This means we will need to build a local paddrs[] array, and after
filling a fs block, do the checksum calculation.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/btrfs_inode.h | 6 ++--
fs/btrfs/file-item.c | 15 +++++++--
fs/btrfs/inode.c | 73 ++++++++++++++++++++++++++++++------------
3 files changed, 68 insertions(+), 26 deletions(-)
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f45dcdde0efc..03c107ab391b 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -542,8 +542,10 @@ static inline void btrfs_set_inode_mapping_order(struct btrfs_inode *inode)
#define CSUM_FMT "0x%*phN"
#define CSUM_FMT_VALUE(size, bytes) size, bytes
-void btrfs_calculate_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr,
- u8 *dest);
+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_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index a42e6d54e7cd..5b3c5489ee48 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -778,6 +778,10 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio)
struct bvec_iter iter = bio->bi_iter;
phys_addr_t paddr;
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;
unsigned nofs_flag;
@@ -797,9 +801,14 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio)
shash->tfm = fs_info->csum_shash;
- btrfs_bio_for_each_block(paddr, bio, &iter, blocksize) {
- btrfs_calculate_block_csum(fs_info, paddr, sums->sums + index);
- index += fs_info->csum_size;
+ btrfs_bio_for_each_block(paddr, bio, &iter, step) {
+ paddrs[(offset / step) % nr_steps] = paddr;
+ offset += step;
+
+ if (IS_ALIGNED(offset, blocksize)) {
+ btrfs_calculate_block_csum_pages(fs_info, paddrs, sums->sums + index);
+ index += fs_info->csum_size;
+ }
}
bbio->sums = sums;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ac2fd589697d..287c67296a7d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3330,36 +3330,67 @@ int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered)
return btrfs_finish_one_ordered(ordered);
}
-void btrfs_calculate_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr,
- u8 *dest)
+/*
+ * Calculate the checksum of a fs block at physical memory address @paddr,
+ * and save the result to @dest.
+ *
+ * The folio containing @paddr must be large enough to contain a full fs block.
+ */
+void btrfs_calculate_block_csum_folio(struct btrfs_fs_info *fs_info,
+ const phys_addr_t paddr, u8 *dest)
{
struct folio *folio = page_folio(phys_to_page(paddr));
const u32 blocksize = fs_info->sectorsize;
- SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
+ const u32 step = min(blocksize, PAGE_SIZE);
+ const u32 nr_steps = blocksize / step;
+ phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE / PAGE_SIZE];
- shash->tfm = fs_info->csum_shash;
/* The full block must be inside the folio. */
ASSERT(offset_in_folio(folio, paddr) + blocksize <= folio_size(folio));
- if (folio_test_partial_kmap(folio)) {
- size_t cur = paddr;
+ for (int i = 0; i < nr_steps; i++) {
+ u32 pindex = offset_in_folio(folio, paddr + i * step) >> PAGE_SHIFT;
- crypto_shash_init(shash);
- while (cur < paddr + blocksize) {
- void *kaddr;
- size_t len = min(paddr + blocksize - cur,
- PAGE_SIZE - offset_in_page(cur));
-
- kaddr = kmap_local_folio(folio, offset_in_folio(folio, cur));
- crypto_shash_update(shash, kaddr, len);
- kunmap_local(kaddr);
- cur += len;
- }
- crypto_shash_final(shash, dest);
- } else {
- crypto_shash_digest(shash, phys_to_virt(paddr), blocksize, dest);
+ /*
+ * For bs <= ps cases, we will only run the loop once, so the offset
+ * inside the page will only added to paddrs[0].
+ *
+ * For bs > ps cases, the block must be page aligned, thus offset
+ * inside the page will always be 0.
+ */
+ paddrs[i] = page_to_phys(folio_page(folio, pindex)) + offset_in_page(paddr);
}
+ return btrfs_calculate_block_csum_pages(fs_info, paddrs, dest);
}
+
+/*
+ * Calculate the checksum of a fs block backed by multiple incontiguous pages at @paddrs[]
+ * and save the result to @dest.
+ *
+ * The folio containing @paddr must be large enough to contain a full fs block.
+ */
+void btrfs_calculate_block_csum_pages(struct btrfs_fs_info *fs_info,
+ const phys_addr_t paddrs[], u8 *dest)
+{
+ const u32 blocksize = fs_info->sectorsize;
+ const u32 step = min(blocksize, PAGE_SIZE);
+ const u32 nr_steps = blocksize / step;
+ SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
+
+ shash->tfm = fs_info->csum_shash;
+ crypto_shash_init(shash);
+ for (int i = 0; i < nr_steps; i++) {
+ const phys_addr_t paddr = paddrs[i];
+ void *kaddr;
+
+ ASSERT(offset_in_page(paddr) + step <= PAGE_SIZE);
+ kaddr = kmap_local_page(phys_to_page(paddr)) + offset_in_page(paddr);
+ crypto_shash_update(shash, kaddr, step);
+ kunmap_local(kaddr);
+ }
+ crypto_shash_final(shash, dest);
+}
+
/*
* Verify the checksum for a single sector without any extra action that depend
* on the type of I/O.
@@ -3369,7 +3400,7 @@ void btrfs_calculate_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr
int btrfs_check_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr, u8 *csum,
const u8 * const csum_expected)
{
- btrfs_calculate_block_csum(fs_info, paddr, csum);
+ btrfs_calculate_block_csum_folio(fs_info, paddr, csum);
if (unlikely(memcmp(csum, csum_expected, fs_info->csum_size) != 0))
return -EIO;
return 0;
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-01 9:50 [PATCH 0/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
2025-10-01 9:50 ` [PATCH 1/4] btrfs: make btrfs_csum_one_bio() handle bs > ps without large folios Qu Wenruo
@ 2025-10-01 9:50 ` Qu Wenruo
2025-10-02 16:46 ` kernel test robot
2025-10-03 7:58 ` Dan Carpenter
2025-10-01 9:50 ` [PATCH 3/4] btrfs: make read verification " Qu Wenruo
2025-10-01 9:50 ` [PATCH 4/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
3 siblings, 2 replies; 10+ messages in thread
From: Qu Wenruo @ 2025-10-01 9:50 UTC (permalink / raw)
To: linux-btrfs
Currently btrfs_repair_io_failure() only accept a single @paddr
parameter, and for bs > ps cases it's required that @paddr is backed by
a large folio.
That assumption has quite some limits, preventing us from utilizing true
zero-copy direct-io and encoded read/writes.
To address the problem, enhance btrfs_repair_io_failure() by:
- Accept an array of paddrs, up to 64K / PAGE_SIZE entries
This kind acts like a bio_vec, but with very limited entries, as the
function is only utilized to repair one fs data block, or a tree block.
Both has a upper size limit (BTRFS_MAX_BLOCK_SIZE, aka 64K), so we
don't need the full bio_vec thing to handle it.
- Allocate a bio with multiple slots
Previously even for bs > ps cases, we only pass in a contiguous
physical address range, thus a single slot will be enough.
But not any more, so we have to allocate a bio structure, other than
using the on-stack one.
- Use on-stack memory to allocate @paddrs array
It's at most 16 pages (4K page size, 64K block size), will take up at
most 128 bytes.
I think the on-stack cost is still acceptable.
- Add one extra check to make sure the repair bio is exactly one block
- Utilize btrfs_repair_io_failure() to submit a single bio for metadata
This should improve the read-repair performance for metadata, as now
we submit a node sized bio then wait, other than submit each block of
the metadata and wait for each submitted block.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/bio.c | 68 ++++++++++++++++++++++++++++++++++++----------
fs/btrfs/bio.h | 2 +-
fs/btrfs/disk-io.c | 29 ++++++++++++--------
3 files changed, 73 insertions(+), 26 deletions(-)
diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 21df48e6c4fa..be5049d3df64 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -164,7 +164,21 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
struct btrfs_inode *inode = repair_bbio->inode;
struct btrfs_fs_info *fs_info = inode->root->fs_info;
struct bio_vec *bv = bio_first_bvec_all(&repair_bbio->bio);
+ /*
+ * 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 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];
+ phys_addr_t paddr;
+ unsigned int slot = 0;
+
+ /* Repair bbio should be eaxctly one block sized. */
+ ASSERT(repair_bbio->saved_iter.bi_size == fs_info->sectorsize);
if (repair_bbio->bio.bi_status ||
!btrfs_data_csum_ok(repair_bbio, dev, 0, bvec_phys(bv))) {
@@ -182,12 +196,17 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
return;
}
+ btrfs_bio_for_each_block(paddr, &repair_bbio->bio, &saved_iter, step) {
+ ASSERT(slot < nr_steps);
+ paddrs[slot] = paddr;
+ slot++;
+ }
+
do {
mirror = prev_repair_mirror(fbio, mirror);
btrfs_repair_io_failure(fs_info, btrfs_ino(inode),
repair_bbio->file_offset, fs_info->sectorsize,
- repair_bbio->saved_iter.bi_sector << SECTOR_SHIFT,
- bvec_phys(bv), mirror);
+ logical, paddrs, mirror);
} while (mirror != fbio->bbio->mirror_num);
done:
@@ -824,18 +843,32 @@ 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
+ * @length: Length of the repair write
+ * @logical: Logical address of the range
+ * @paddrs: Physical address array of the content.
+ * Length for each paddr should be min(sectorsize, PAGE_SIZE).
+ * @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 start,
- u64 length, u64 logical, phys_addr_t paddr, int mirror_num)
+int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff,
+ u64 length, u64 logical, const phys_addr_t paddrs[], int mirror_num)
{
+ const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
+ const u32 nr_steps = DIV_ROUND_UP_POW2(length, step);
struct btrfs_io_stripe smap = { 0 };
- struct bio_vec bvec;
- struct bio bio;
+ struct bio *bio = NULL;
int ret = 0;
ASSERT(!(fs_info->sb->s_flags & SB_RDONLY));
BUG_ON(!mirror_num);
+ /* Basic alignment checks. */
+ ASSERT(IS_ALIGNED(logical, fs_info->sectorsize));
+ ASSERT(IS_ALIGNED(length, fs_info->sectorsize));
+ ASSERT(IS_ALIGNED(fileoff, fs_info->sectorsize));
+
if (btrfs_repair_one_zone(fs_info, logical))
return 0;
@@ -855,24 +888,31 @@ int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 start,
goto out_counter_dec;
}
- bio_init(&bio, smap.dev->bdev, &bvec, 1, REQ_OP_WRITE | REQ_SYNC);
- bio.bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
- __bio_add_page(&bio, phys_to_page(paddr), length, offset_in_page(paddr));
- ret = submit_bio_wait(&bio);
+ bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
+ /* Backed by fs_bio_set, shouldn't fail. */
+ ASSERT(bio);
+ 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);
+ }
+ ret = submit_bio_wait(bio);
if (ret) {
/* try to remap that extent elsewhere? */
btrfs_dev_stat_inc_and_print(smap.dev, BTRFS_DEV_STAT_WRITE_ERRS);
- goto out_bio_uninit;
+ goto out_free_bio;
}
btrfs_info_rl(fs_info,
"read error corrected: ino %llu off %llu (dev %s sector %llu)",
- ino, start, btrfs_dev_name(smap.dev),
+ ino, fileoff, btrfs_dev_name(smap.dev),
smap.physical >> SECTOR_SHIFT);
ret = 0;
-out_bio_uninit:
- bio_uninit(&bio);
+out_free_bio:
+ if (bio)
+ bio_put(bio);
out_counter_dec:
btrfs_bio_counter_dec(fs_info);
return ret;
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 00883aea55d7..c5e2e059d2f0 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -112,6 +112,6 @@ 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 start,
- u64 length, u64 logical, phys_addr_t paddr, int mirror_num);
+ u64 length, u64 logical, const phys_addr_t paddrs[], int mirror_num);
#endif
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f475fb2272ac..2623b84dd6ed 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -182,26 +182,33 @@ 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];
int ret = 0;
if (sb_rdonly(fs_info->sb))
return -EROFS;
- for (int i = 0; i < num_extent_folios(eb); i++) {
+ for (int i = 0; i < num_extent_pages(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,
- folio_pos(folio) + eb->folio_size);
- u32 len = end - start;
- phys_addr_t paddr = PFN_PHYS(folio_pfn(folio)) +
- offset_in_folio(folio, start);
- ret = btrfs_repair_io_failure(fs_info, 0, start, len, start,
- paddr, mirror_num);
- if (ret)
- break;
+ /* 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
+ * offset inside the page.
+ *
+ * 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 = btrfs_repair_io_failure(fs_info, 0, eb->start, eb->len, eb->start,
+ paddrs, mirror_num);
return ret;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-01 9:50 ` [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases " Qu Wenruo
@ 2025-10-02 16:46 ` kernel test robot
2025-10-03 7:58 ` Dan Carpenter
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-10-02 16:46 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: oe-kbuild-all
Hi Qu,
kernel test robot noticed the following build errors:
[auto build test ERROR on kdave/for-next]
[also build test ERROR on linus/master next-20250929]
[cannot apply to v6.17]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-make-btrfs_csum_one_bio-handle-bs-ps-without-large-folios/20251001-175128
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/33c39907866c148a360ff60387097fbad63a19aa.1759311101.git.wqu%40suse.com
patch subject: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
config: i386-randconfig-011-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030041.mTtTa2Pr-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.4.0-5) 12.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251003/202510030041.mTtTa2Pr-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510030041.mTtTa2Pr-lkp@intel.com/
All errors (new ones prefixed by >>):
ld: fs/btrfs/bio.o: in function `btrfs_repair_io_failure':
>> fs/btrfs/bio.c:859:(.text+0x188c): undefined reference to `__udivdi3'
vim +859 fs/btrfs/bio.c
836
837 /*
838 * Submit a repair write.
839 *
840 * This bypasses btrfs_submit_bbio() deliberately, as that writes all copies in a
841 * RAID setup. Here we only want to write the one bad copy, so we do the
842 * mapping ourselves and submit the bio directly.
843 *
844 * The I/O is issued synchronously to block the repair read completion from
845 * freeing the bio.
846 *
847 * @ino: Offending inode number
848 * @fileoff: File offset inside the inode
849 * @length: Length of the repair write
850 * @logical: Logical address of the range
851 * @paddrs: Physical address array of the content.
852 * Length for each paddr should be min(sectorsize, PAGE_SIZE).
853 * @mirror_num: Mirror number to write to. Must not be zero.
854 */
855 int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff,
856 u64 length, u64 logical, const phys_addr_t paddrs[], int mirror_num)
857 {
858 const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
> 859 const u32 nr_steps = DIV_ROUND_UP_POW2(length, step);
860 struct btrfs_io_stripe smap = { 0 };
861 struct bio *bio = NULL;
862 int ret = 0;
863
864 ASSERT(!(fs_info->sb->s_flags & SB_RDONLY));
865 BUG_ON(!mirror_num);
866
867 /* Basic alignment checks. */
868 ASSERT(IS_ALIGNED(logical, fs_info->sectorsize));
869 ASSERT(IS_ALIGNED(length, fs_info->sectorsize));
870 ASSERT(IS_ALIGNED(fileoff, fs_info->sectorsize));
871
872 if (btrfs_repair_one_zone(fs_info, logical))
873 return 0;
874
875 /*
876 * Avoid races with device replace and make sure our bioc has devices
877 * associated to its stripes that don't go away while we are doing the
878 * read repair operation.
879 */
880 btrfs_bio_counter_inc_blocked(fs_info);
881 ret = btrfs_map_repair_block(fs_info, &smap, logical, length, mirror_num);
882 if (ret < 0)
883 goto out_counter_dec;
884
885 if (unlikely(!smap.dev->bdev ||
886 !test_bit(BTRFS_DEV_STATE_WRITEABLE, &smap.dev->dev_state))) {
887 ret = -EIO;
888 goto out_counter_dec;
889 }
890
891 bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
892 /* Backed by fs_bio_set, shouldn't fail. */
893 ASSERT(bio);
894 bio->bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
895 for (int i = 0; i < nr_steps; i++) {
896 ret = bio_add_page(bio, phys_to_page(paddrs[i]), step, offset_in_page(paddrs[i]));
897 /* We should have allocated enough slots to contain all the different pages. */
898 ASSERT(ret == step);
899 }
900 ret = submit_bio_wait(bio);
901 if (ret) {
902 /* try to remap that extent elsewhere? */
903 btrfs_dev_stat_inc_and_print(smap.dev, BTRFS_DEV_STAT_WRITE_ERRS);
904 goto out_free_bio;
905 }
906
907 btrfs_info_rl(fs_info,
908 "read error corrected: ino %llu off %llu (dev %s sector %llu)",
909 ino, fileoff, btrfs_dev_name(smap.dev),
910 smap.physical >> SECTOR_SHIFT);
911 ret = 0;
912
913 out_free_bio:
914 if (bio)
915 bio_put(bio);
916 out_counter_dec:
917 btrfs_bio_counter_dec(fs_info);
918 return ret;
919 }
920
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-01 9:50 ` [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases " Qu Wenruo
2025-10-02 16:46 ` kernel test robot
@ 2025-10-03 7:58 ` Dan Carpenter
2025-10-03 9:17 ` Qu Wenruo
1 sibling, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2025-10-03 7:58 UTC (permalink / raw)
To: oe-kbuild, Qu Wenruo, linux-btrfs; +Cc: lkp, oe-kbuild-all
Hi Qu,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-make-btrfs_csum_one_bio-handle-bs-ps-without-large-folios/20251001-175128
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
patch link: https://lore.kernel.org/r/33c39907866c148a360ff60387097fbad63a19aa.1759311101.git.wqu%40suse.com
patch subject: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
config: powerpc64-randconfig-r071-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030550.mqFoO0Dw-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 39f292ffa13d7ca0d1edff27ac8fd55024bb4d19)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202510030550.mqFoO0Dw-lkp@intel.com/
smatch warnings:
fs/btrfs/bio.c:914 btrfs_repair_io_failure() warn: variable dereferenced before check 'bio' (see line 894)
vim +/bio +914 fs/btrfs/bio.c
2d65a91734a195 Qu Wenruo 2025-10-01 855 int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff,
2d65a91734a195 Qu Wenruo 2025-10-01 856 u64 length, u64 logical, const phys_addr_t paddrs[], int mirror_num)
bacf60e5158629 Christoph Hellwig 2022-11-15 857 {
2d65a91734a195 Qu Wenruo 2025-10-01 858 const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
2d65a91734a195 Qu Wenruo 2025-10-01 859 const u32 nr_steps = DIV_ROUND_UP_POW2(length, step);
4886ff7b50f634 Qu Wenruo 2023-03-20 860 struct btrfs_io_stripe smap = { 0 };
2d65a91734a195 Qu Wenruo 2025-10-01 861 struct bio *bio = NULL;
bacf60e5158629 Christoph Hellwig 2022-11-15 862 int ret = 0;
bacf60e5158629 Christoph Hellwig 2022-11-15 863
bacf60e5158629 Christoph Hellwig 2022-11-15 864 ASSERT(!(fs_info->sb->s_flags & SB_RDONLY));
bacf60e5158629 Christoph Hellwig 2022-11-15 865 BUG_ON(!mirror_num);
bacf60e5158629 Christoph Hellwig 2022-11-15 866
2d65a91734a195 Qu Wenruo 2025-10-01 867 /* Basic alignment checks. */
2d65a91734a195 Qu Wenruo 2025-10-01 868 ASSERT(IS_ALIGNED(logical, fs_info->sectorsize));
2d65a91734a195 Qu Wenruo 2025-10-01 869 ASSERT(IS_ALIGNED(length, fs_info->sectorsize));
2d65a91734a195 Qu Wenruo 2025-10-01 870 ASSERT(IS_ALIGNED(fileoff, fs_info->sectorsize));
2d65a91734a195 Qu Wenruo 2025-10-01 871
bacf60e5158629 Christoph Hellwig 2022-11-15 872 if (btrfs_repair_one_zone(fs_info, logical))
bacf60e5158629 Christoph Hellwig 2022-11-15 873 return 0;
bacf60e5158629 Christoph Hellwig 2022-11-15 874
bacf60e5158629 Christoph Hellwig 2022-11-15 875 /*
bacf60e5158629 Christoph Hellwig 2022-11-15 876 * Avoid races with device replace and make sure our bioc has devices
bacf60e5158629 Christoph Hellwig 2022-11-15 877 * associated to its stripes that don't go away while we are doing the
bacf60e5158629 Christoph Hellwig 2022-11-15 878 * read repair operation.
bacf60e5158629 Christoph Hellwig 2022-11-15 879 */
bacf60e5158629 Christoph Hellwig 2022-11-15 880 btrfs_bio_counter_inc_blocked(fs_info);
4886ff7b50f634 Qu Wenruo 2023-03-20 881 ret = btrfs_map_repair_block(fs_info, &smap, logical, length, mirror_num);
4886ff7b50f634 Qu Wenruo 2023-03-20 882 if (ret < 0)
d73a27b86fc722 Qu Wenruo 2023-01-01 883 goto out_counter_dec;
bacf60e5158629 Christoph Hellwig 2022-11-15 884
cc53bd2085c8fa David Sterba 2025-09-17 885 if (unlikely(!smap.dev->bdev ||
cc53bd2085c8fa David Sterba 2025-09-17 886 !test_bit(BTRFS_DEV_STATE_WRITEABLE, &smap.dev->dev_state))) {
bacf60e5158629 Christoph Hellwig 2022-11-15 887 ret = -EIO;
bacf60e5158629 Christoph Hellwig 2022-11-15 888 goto out_counter_dec;
bacf60e5158629 Christoph Hellwig 2022-11-15 889 }
bacf60e5158629 Christoph Hellwig 2022-11-15 890
2d65a91734a195 Qu Wenruo 2025-10-01 891 bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
2d65a91734a195 Qu Wenruo 2025-10-01 892 /* Backed by fs_bio_set, shouldn't fail. */
2d65a91734a195 Qu Wenruo 2025-10-01 893 ASSERT(bio);
2d65a91734a195 Qu Wenruo 2025-10-01 @894 bio->bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
2d65a91734a195 Qu Wenruo 2025-10-01 895 for (int i = 0; i < nr_steps; i++) {
2d65a91734a195 Qu Wenruo 2025-10-01 896 ret = bio_add_page(bio, phys_to_page(paddrs[i]), step, offset_in_page(paddrs[i]));
2d65a91734a195 Qu Wenruo 2025-10-01 897 /* We should have allocated enough slots to contain all the different pages. */
2d65a91734a195 Qu Wenruo 2025-10-01 898 ASSERT(ret == step);
2d65a91734a195 Qu Wenruo 2025-10-01 899 }
2d65a91734a195 Qu Wenruo 2025-10-01 900 ret = submit_bio_wait(bio);
bacf60e5158629 Christoph Hellwig 2022-11-15 901 if (ret) {
bacf60e5158629 Christoph Hellwig 2022-11-15 902 /* try to remap that extent elsewhere? */
4886ff7b50f634 Qu Wenruo 2023-03-20 903 btrfs_dev_stat_inc_and_print(smap.dev, BTRFS_DEV_STAT_WRITE_ERRS);
2d65a91734a195 Qu Wenruo 2025-10-01 904 goto out_free_bio;
bacf60e5158629 Christoph Hellwig 2022-11-15 905 }
bacf60e5158629 Christoph Hellwig 2022-11-15 906
2eac2ae8b214ab David Sterba 2025-06-09 907 btrfs_info_rl(fs_info,
bacf60e5158629 Christoph Hellwig 2022-11-15 908 "read error corrected: ino %llu off %llu (dev %s sector %llu)",
2d65a91734a195 Qu Wenruo 2025-10-01 909 ino, fileoff, btrfs_dev_name(smap.dev),
4886ff7b50f634 Qu Wenruo 2023-03-20 910 smap.physical >> SECTOR_SHIFT);
bacf60e5158629 Christoph Hellwig 2022-11-15 911 ret = 0;
bacf60e5158629 Christoph Hellwig 2022-11-15 912
2d65a91734a195 Qu Wenruo 2025-10-01 913 out_free_bio:
2d65a91734a195 Qu Wenruo 2025-10-01 @914 if (bio)
This check could be deleted if you want. Or you could leave it since
it's harmless. Up to you. I would prefer to silence these warning by
updating Smatch, but Smatch isn't smart enough to parse bio_alloc()
Smatch deliberately ignores ASSERT()s.
2d65a91734a195 Qu Wenruo 2025-10-01 915 bio_put(bio);
bacf60e5158629 Christoph Hellwig 2022-11-15 916 out_counter_dec:
bacf60e5158629 Christoph Hellwig 2022-11-15 917 btrfs_bio_counter_dec(fs_info);
bacf60e5158629 Christoph Hellwig 2022-11-15 918 return ret;
bacf60e5158629 Christoph Hellwig 2022-11-15 919 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-03 7:58 ` Dan Carpenter
@ 2025-10-03 9:17 ` Qu Wenruo
2025-10-03 10:53 ` Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2025-10-03 9:17 UTC (permalink / raw)
To: Dan Carpenter, oe-kbuild, Qu Wenruo, linux-btrfs; +Cc: lkp, oe-kbuild-all
在 2025/10/3 17:28, Dan Carpenter 写道:
> Hi Qu,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Qu-Wenruo/btrfs-make-btrfs_csum_one_bio-handle-bs-ps-without-large-folios/20251001-175128
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> patch link: https://lore.kernel.org/r/33c39907866c148a360ff60387097fbad63a19aa.1759311101.git.wqu%40suse.com
> patch subject: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
> config: powerpc64-randconfig-r071-20251002 (https://download.01.org/0day-ci/archive/20251003/202510030550.mqFoO0Dw-lkp@intel.com/config)
> compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 39f292ffa13d7ca0d1edff27ac8fd55024bb4d19)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202510030550.mqFoO0Dw-lkp@intel.com/
>
> smatch warnings:
> fs/btrfs/bio.c:914 btrfs_repair_io_failure() warn: variable dereferenced before check 'bio' (see line 894)
>
> vim +/bio +914 fs/btrfs/bio.c
>
> 2d65a91734a195 Qu Wenruo 2025-10-01 855 int btrfs_repair_io_failure(struct btrfs_fs_info *fs_info, u64 ino, u64 fileoff,
> 2d65a91734a195 Qu Wenruo 2025-10-01 856 u64 length, u64 logical, const phys_addr_t paddrs[], int mirror_num)
> bacf60e5158629 Christoph Hellwig 2022-11-15 857 {
> 2d65a91734a195 Qu Wenruo 2025-10-01 858 const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
> 2d65a91734a195 Qu Wenruo 2025-10-01 859 const u32 nr_steps = DIV_ROUND_UP_POW2(length, step);
> 4886ff7b50f634 Qu Wenruo 2023-03-20 860 struct btrfs_io_stripe smap = { 0 };
> 2d65a91734a195 Qu Wenruo 2025-10-01 861 struct bio *bio = NULL;
> bacf60e5158629 Christoph Hellwig 2022-11-15 862 int ret = 0;
> bacf60e5158629 Christoph Hellwig 2022-11-15 863
> bacf60e5158629 Christoph Hellwig 2022-11-15 864 ASSERT(!(fs_info->sb->s_flags & SB_RDONLY));
> bacf60e5158629 Christoph Hellwig 2022-11-15 865 BUG_ON(!mirror_num);
> bacf60e5158629 Christoph Hellwig 2022-11-15 866
> 2d65a91734a195 Qu Wenruo 2025-10-01 867 /* Basic alignment checks. */
> 2d65a91734a195 Qu Wenruo 2025-10-01 868 ASSERT(IS_ALIGNED(logical, fs_info->sectorsize));
> 2d65a91734a195 Qu Wenruo 2025-10-01 869 ASSERT(IS_ALIGNED(length, fs_info->sectorsize));
> 2d65a91734a195 Qu Wenruo 2025-10-01 870 ASSERT(IS_ALIGNED(fileoff, fs_info->sectorsize));
> 2d65a91734a195 Qu Wenruo 2025-10-01 871
> bacf60e5158629 Christoph Hellwig 2022-11-15 872 if (btrfs_repair_one_zone(fs_info, logical))
> bacf60e5158629 Christoph Hellwig 2022-11-15 873 return 0;
> bacf60e5158629 Christoph Hellwig 2022-11-15 874
> bacf60e5158629 Christoph Hellwig 2022-11-15 875 /*
> bacf60e5158629 Christoph Hellwig 2022-11-15 876 * Avoid races with device replace and make sure our bioc has devices
> bacf60e5158629 Christoph Hellwig 2022-11-15 877 * associated to its stripes that don't go away while we are doing the
> bacf60e5158629 Christoph Hellwig 2022-11-15 878 * read repair operation.
> bacf60e5158629 Christoph Hellwig 2022-11-15 879 */
> bacf60e5158629 Christoph Hellwig 2022-11-15 880 btrfs_bio_counter_inc_blocked(fs_info);
> 4886ff7b50f634 Qu Wenruo 2023-03-20 881 ret = btrfs_map_repair_block(fs_info, &smap, logical, length, mirror_num);
> 4886ff7b50f634 Qu Wenruo 2023-03-20 882 if (ret < 0)
> d73a27b86fc722 Qu Wenruo 2023-01-01 883 goto out_counter_dec;
> bacf60e5158629 Christoph Hellwig 2022-11-15 884
> cc53bd2085c8fa David Sterba 2025-09-17 885 if (unlikely(!smap.dev->bdev ||
> cc53bd2085c8fa David Sterba 2025-09-17 886 !test_bit(BTRFS_DEV_STATE_WRITEABLE, &smap.dev->dev_state))) {
> bacf60e5158629 Christoph Hellwig 2022-11-15 887 ret = -EIO;
> bacf60e5158629 Christoph Hellwig 2022-11-15 888 goto out_counter_dec;
> bacf60e5158629 Christoph Hellwig 2022-11-15 889 }
> bacf60e5158629 Christoph Hellwig 2022-11-15 890
> 2d65a91734a195 Qu Wenruo 2025-10-01 891 bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
> 2d65a91734a195 Qu Wenruo 2025-10-01 892 /* Backed by fs_bio_set, shouldn't fail. */
> 2d65a91734a195 Qu Wenruo 2025-10-01 893 ASSERT(bio);
> 2d65a91734a195 Qu Wenruo 2025-10-01 @894 bio->bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
I have no idea how to make smatch happy.
Maybe goto out_counter_dec for the bio allocation failure branch?
But that will be rejected by human reviews as it's really unnecessary.
> 2d65a91734a195 Qu Wenruo 2025-10-01 895 for (int i = 0; i < nr_steps; i++) {
> 2d65a91734a195 Qu Wenruo 2025-10-01 896 ret = bio_add_page(bio, phys_to_page(paddrs[i]), step, offset_in_page(paddrs[i]));
> 2d65a91734a195 Qu Wenruo 2025-10-01 897 /* We should have allocated enough slots to contain all the different pages. */
> 2d65a91734a195 Qu Wenruo 2025-10-01 898 ASSERT(ret == step);
> 2d65a91734a195 Qu Wenruo 2025-10-01 899 }
> 2d65a91734a195 Qu Wenruo 2025-10-01 900 ret = submit_bio_wait(bio);
> bacf60e5158629 Christoph Hellwig 2022-11-15 901 if (ret) {
> bacf60e5158629 Christoph Hellwig 2022-11-15 902 /* try to remap that extent elsewhere? */
> 4886ff7b50f634 Qu Wenruo 2023-03-20 903 btrfs_dev_stat_inc_and_print(smap.dev, BTRFS_DEV_STAT_WRITE_ERRS);
> 2d65a91734a195 Qu Wenruo 2025-10-01 904 goto out_free_bio;
> bacf60e5158629 Christoph Hellwig 2022-11-15 905 }
> bacf60e5158629 Christoph Hellwig 2022-11-15 906
> 2eac2ae8b214ab David Sterba 2025-06-09 907 btrfs_info_rl(fs_info,
> bacf60e5158629 Christoph Hellwig 2022-11-15 908 "read error corrected: ino %llu off %llu (dev %s sector %llu)",
> 2d65a91734a195 Qu Wenruo 2025-10-01 909 ino, fileoff, btrfs_dev_name(smap.dev),
> 4886ff7b50f634 Qu Wenruo 2023-03-20 910 smap.physical >> SECTOR_SHIFT);
> bacf60e5158629 Christoph Hellwig 2022-11-15 911 ret = 0;
> bacf60e5158629 Christoph Hellwig 2022-11-15 912
> 2d65a91734a195 Qu Wenruo 2025-10-01 913 out_free_bio:
> 2d65a91734a195 Qu Wenruo 2025-10-01 @914 if (bio)
>
> This check could be deleted if you want.
I can move the bio_put() call immediately after the submit_bio_wait()
call, as that's also the pattern used in other locations like
xfs_rw_bdev() and squashfs_bio_read_cached().
Thanks,
Qu
> Or you could leave it since
> it's harmless. Up to you. I would prefer to silence these warning by
> updating Smatch, but Smatch isn't smart enough to parse bio_alloc()
> Smatch deliberately ignores ASSERT()s.
>
> 2d65a91734a195 Qu Wenruo 2025-10-01 915 bio_put(bio);
> bacf60e5158629 Christoph Hellwig 2022-11-15 916 out_counter_dec:
> bacf60e5158629 Christoph Hellwig 2022-11-15 917 btrfs_bio_counter_dec(fs_info);
> bacf60e5158629 Christoph Hellwig 2022-11-15 918 return ret;
> bacf60e5158629 Christoph Hellwig 2022-11-15 919 }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-03 9:17 ` Qu Wenruo
@ 2025-10-03 10:53 ` Dan Carpenter
2025-10-03 21:18 ` Qu Wenruo
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2025-10-03 10:53 UTC (permalink / raw)
To: Qu Wenruo; +Cc: oe-kbuild, Qu Wenruo, linux-btrfs, lkp, oe-kbuild-all
On Fri, Oct 03, 2025 at 06:47:11PM +0930, Qu Wenruo wrote:
> > cc53bd2085c8fa David Sterba 2025-09-17 885 if (unlikely(!smap.dev->bdev ||
> > cc53bd2085c8fa David Sterba 2025-09-17 886 !test_bit(BTRFS_DEV_STATE_WRITEABLE, &smap.dev->dev_state))) {
> > bacf60e5158629 Christoph Hellwig 2022-11-15 887 ret = -EIO;
> > bacf60e5158629 Christoph Hellwig 2022-11-15 888 goto out_counter_dec;
> > bacf60e5158629 Christoph Hellwig 2022-11-15 889 }
> > bacf60e5158629 Christoph Hellwig 2022-11-15 890
> > 2d65a91734a195 Qu Wenruo 2025-10-01 891 bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
> > 2d65a91734a195 Qu Wenruo 2025-10-01 892 /* Backed by fs_bio_set, shouldn't fail. */
> > 2d65a91734a195 Qu Wenruo 2025-10-01 893 ASSERT(bio);
> > 2d65a91734a195 Qu Wenruo 2025-10-01 @894 bio->bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
>
> I have no idea how to make smatch happy.
I would just delete the unnecessary NULL check, but an equally valid
thing is to just ignore the warning. In the kernel, all old warnings
are false positives. I'm never aiming for being completely free from
static checker warnings because that means we have to be less ambitious
about what we check for.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases without large folios
2025-10-03 10:53 ` Dan Carpenter
@ 2025-10-03 21:18 ` Qu Wenruo
0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2025-10-03 21:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, Qu Wenruo, linux-btrfs, lkp, oe-kbuild-all
在 2025/10/3 20:23, Dan Carpenter 写道:
> On Fri, Oct 03, 2025 at 06:47:11PM +0930, Qu Wenruo wrote:
>>> cc53bd2085c8fa David Sterba 2025-09-17 885 if (unlikely(!smap.dev->bdev ||
>>> cc53bd2085c8fa David Sterba 2025-09-17 886 !test_bit(BTRFS_DEV_STATE_WRITEABLE, &smap.dev->dev_state))) {
>>> bacf60e5158629 Christoph Hellwig 2022-11-15 887 ret = -EIO;
>>> bacf60e5158629 Christoph Hellwig 2022-11-15 888 goto out_counter_dec;
>>> bacf60e5158629 Christoph Hellwig 2022-11-15 889 }
>>> bacf60e5158629 Christoph Hellwig 2022-11-15 890
>>> 2d65a91734a195 Qu Wenruo 2025-10-01 891 bio = bio_alloc(smap.dev->bdev, nr_steps, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS);
>>> 2d65a91734a195 Qu Wenruo 2025-10-01 892 /* Backed by fs_bio_set, shouldn't fail. */
>>> 2d65a91734a195 Qu Wenruo 2025-10-01 893 ASSERT(bio);
>>> 2d65a91734a195 Qu Wenruo 2025-10-01 @894 bio->bi_iter.bi_sector = smap.physical >> SECTOR_SHIFT;
>>
>> I have no idea how to make smatch happy.
>
> I would just delete the unnecessary NULL check,
Sounds good, as all other fses are doing without a NULL check.
Thanks,
Qu
> but an equally valid
> thing is to just ignore the warning. In the kernel, all old warnings
> are false positives. I'm never aiming for being completely free from
> static checker warnings because that means we have to be less ambitious
> about what we check for.
>
> regards,
> dan carpenter
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] btrfs: make read verification handle bs > ps cases without large folios
2025-10-01 9:50 [PATCH 0/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
2025-10-01 9:50 ` [PATCH 1/4] btrfs: make btrfs_csum_one_bio() handle bs > ps without large folios Qu Wenruo
2025-10-01 9:50 ` [PATCH 2/4] btrfs: make btrfs_repair_io_failure() handle bs > ps cases " Qu Wenruo
@ 2025-10-01 9:50 ` Qu Wenruo
2025-10-01 9:50 ` [PATCH 4/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
3 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2025-10-01 9:50 UTC (permalink / raw)
To: linux-btrfs
The current read verification is also relying on large folios to support
bs > ps cases, but that introduced quite some limits.
To enhance read-repair to support bs > ps without large folios:
- Make btrfs_data_csum_ok() to accept an array of paddrs
Which can pass the paddrs[] direct into
btrfs_calculate_block_csum_pages().
- Make repair_one_sector() to accept an array of paddrs
So that it can submit a repair bio backed by regular pages, not only
large folios.
This requires us to allocate more slots at bio allocation time though.
Also since the caller may have only partially advanced the saved_iter
for bs > ps cases, we can not directly trust the logical bytenr from
saved_iter (can be unaligned), thus a manual round down is necessary
for the logical bytenr.
- Make btrfs_check_read_bio() to build an array of paddrs
The tricky part is that we can only call btrfs_data_csum_ok() after
all involved pages are assembled.
This means at the call time of btrfs_check_read_bio(), our offset
inside the bio is already at the end of the fs block.
Thus we must re-calculate @bio_offset for btrfs_data_csum_ok() and
repair_one_sector().
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/bio.c | 62 ++++++++++++++++++++++++++++--------------
fs/btrfs/btrfs_inode.h | 2 +-
fs/btrfs/inode.c | 18 ++++++------
3 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index be5049d3df64..a29812d3d2c9 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -163,7 +163,6 @@ 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;
- struct bio_vec *bv = bio_first_bvec_all(&repair_bbio->bio);
/*
* We can not move forward the saved_iter, as it will be later
* utilized by repair_bbio again.
@@ -180,8 +179,14 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
/* Repair bbio should be eaxctly 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, bvec_phys(bv))) {
+ !btrfs_data_csum_ok(repair_bbio, dev, 0, paddrs)) {
bio_reset(&repair_bbio->bio, NULL, REQ_OP_READ);
repair_bbio->bio.bi_iter = repair_bbio->saved_iter;
@@ -196,12 +201,6 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
return;
}
- btrfs_bio_for_each_block(paddr, &repair_bbio->bio, &saved_iter, step) {
- ASSERT(slot < nr_steps);
- paddrs[slot] = paddr;
- slot++;
- }
-
do {
mirror = prev_repair_mirror(fbio, mirror);
btrfs_repair_io_failure(fs_info, btrfs_ino(inode),
@@ -223,21 +222,25 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
*/
static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,
u32 bio_offset,
- phys_addr_t paddr,
+ phys_addr_t paddrs[],
struct btrfs_failed_bio *fbio)
{
struct btrfs_inode *inode = failed_bbio->inode;
struct btrfs_fs_info *fs_info = inode->root->fs_info;
- struct folio *folio = page_folio(phys_to_page(paddr));
const u32 sectorsize = fs_info->sectorsize;
- const u32 foff = offset_in_folio(folio, paddr);
- const u64 logical = (failed_bbio->saved_iter.bi_sector << SECTOR_SHIFT);
+ 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;
int num_copies;
int mirror;
- ASSERT(foff + sectorsize <= folio_size(folio));
btrfs_debug(fs_info, "repair read error: read error at %llu",
failed_bbio->file_offset + bio_offset);
@@ -257,10 +260,18 @@ static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,
atomic_inc(&fbio->repair_count);
- repair_bio = bio_alloc_bioset(NULL, 1, REQ_OP_READ, GFP_NOFS,
+ repair_bio = bio_alloc_bioset(NULL, nr_steps, REQ_OP_READ, GFP_NOFS,
&btrfs_repair_bioset);
- repair_bio->bi_iter.bi_sector = failed_bbio->saved_iter.bi_sector;
- bio_add_folio_nofail(repair_bio, folio, sectorsize, foff);
+ repair_bio->bi_iter.bi_sector = logical >> SECTOR_SHIFT;
+ for (int i = 0; i < nr_steps; i++) {
+ int ret;
+
+ ASSERT(offset_in_page(paddrs[i]) + step <= PAGE_SIZE);
+
+ ret = bio_add_page(repair_bio, phys_to_page(paddrs[i]), step,
+ offset_in_page(paddrs[i]));
+ ASSERT(ret == step);
+ }
repair_bbio = btrfs_bio(repair_bio);
btrfs_bio_init(repair_bbio, fs_info, NULL, fbio);
@@ -277,10 +288,13 @@ static void btrfs_check_read_bio(struct btrfs_bio *bbio, struct btrfs_device *de
{
struct btrfs_inode *inode = bbio->inode;
struct btrfs_fs_info *fs_info = inode->root->fs_info;
- u32 sectorsize = fs_info->sectorsize;
+ 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;
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;
@@ -299,10 +313,16 @@ 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, fs_info->sectorsize) {
- if (status || !btrfs_data_csum_ok(bbio, dev, offset, paddr))
- fbio = repair_one_sector(bbio, offset, paddr, fbio);
- offset += sectorsize;
+ 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);
+ }
}
if (bbio->csum != bbio->csum_inline)
kfree(bbio->csum);
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 03c107ab391b..ef3f2c1f7a77 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -549,7 +549,7 @@ void btrfs_calculate_block_csum_pages(struct btrfs_fs_info *fs_info,
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, phys_addr_t paddr);
+ u32 bio_offset, const phys_addr_t paddrs[]);
noinline int can_nocow_extent(struct btrfs_inode *inode, u64 offset, u64 *len,
struct btrfs_file_extent *file_extent,
bool nowait);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 287c67296a7d..dbc5fad7b9a1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3407,12 +3407,13 @@ int btrfs_check_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr, u8
}
/*
- * Verify the checksum of a single data sector.
+ * Verify the checksum of a single data sector, which can be scattered at
+ * different incontiguous pages.
*
* @bbio: btrfs_io_bio which contains the csum
* @dev: device the sector is on
* @bio_offset: offset to the beginning of the bio (in bytes)
- * @bv: bio_vec to check
+ * @paddrs: physical addresses which back the fs block
*
* Check if the checksum on a data block is valid. When a checksum mismatch is
* detected, report the error and fill the corrupted range with zero.
@@ -3420,12 +3421,13 @@ int btrfs_check_block_csum(struct btrfs_fs_info *fs_info, phys_addr_t paddr, u8
* Return %true if the sector is ok or had no checksum to start with, else %false.
*/
bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
- u32 bio_offset, phys_addr_t paddr)
+ u32 bio_offset, const phys_addr_t paddrs[])
{
struct btrfs_inode *inode = bbio->inode;
struct btrfs_fs_info *fs_info = inode->root->fs_info;
const u32 blocksize = fs_info->sectorsize;
- struct folio *folio;
+ const u32 step = min(blocksize, PAGE_SIZE);
+ const u32 nr_steps = blocksize / step;
u64 file_offset = bbio->file_offset + bio_offset;
u64 end = file_offset + blocksize - 1;
u8 *csum_expected;
@@ -3445,7 +3447,8 @@ bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
csum_expected = bbio->csum + (bio_offset >> fs_info->sectorsize_bits) *
fs_info->csum_size;
- if (btrfs_check_block_csum(fs_info, paddr, csum, csum_expected))
+ btrfs_calculate_block_csum_pages(fs_info, paddrs, csum);
+ if (unlikely(memcmp(csum, csum_expected, fs_info->csum_size) != 0))
goto zeroit;
return true;
@@ -3454,9 +3457,8 @@ bool btrfs_data_csum_ok(struct btrfs_bio *bbio, struct btrfs_device *dev,
bbio->mirror_num);
if (dev)
btrfs_dev_stat_inc_and_print(dev, BTRFS_DEV_STAT_CORRUPTION_ERRS);
- folio = page_folio(phys_to_page(paddr));
- ASSERT(offset_in_folio(folio, paddr) + blocksize <= folio_size(folio));
- folio_zero_range(folio, offset_in_folio(folio, paddr), blocksize);
+ for (int i = 0; i < nr_steps; i++)
+ memzero_page(phys_to_page(paddrs[i]), offset_in_page(paddrs[i]), step);
return false;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] btrfs: enable encoded read/write/send for bs > ps cases
2025-10-01 9:50 [PATCH 0/4] btrfs: enable encoded read/write/send for bs > ps cases Qu Wenruo
` (2 preceding siblings ...)
2025-10-01 9:50 ` [PATCH 3/4] btrfs: make read verification " Qu Wenruo
@ 2025-10-01 9:50 ` Qu Wenruo
3 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2025-10-01 9:50 UTC (permalink / raw)
To: linux-btrfs
Since the read verification and read repair are all supporting bs > ps
without large folios now, we can enable encoded read/write/send.
Now we can relax the alignment in assert_bbio_alignment() to
min(blocksize, PAGE_SIZE).
But also add the extra blocksize based alignment check for the logical
and length of the bbio.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/bio.c | 22 ++++++++++++----------
fs/btrfs/ioctl.c | 21 ---------------------
fs/btrfs/send.c | 9 +--------
3 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a29812d3d2c9..35c154aff95e 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -825,21 +825,23 @@ static void assert_bbio_alignment(struct btrfs_bio *bbio)
struct bio_vec bvec;
struct bvec_iter iter;
const u32 blocksize = fs_info->sectorsize;
+ const u32 alignment = min(blocksize, PAGE_SIZE);
+ const u64 logical = bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT;
+ const u32 length = bbio->bio.bi_iter.bi_size;
- /* Metadata has no extra bs > ps alignment requirement. */
- if (!is_data_bbio(bbio))
- return;
+ /* The logical and length should still be aligned to blocksize. */
+ ASSERT(IS_ALIGNED(logical, blocksize) && IS_ALIGNED(length, blocksize) &&
+ length != 0, "root=%llu inode=%llu logical=%llu length=%u",
+ btrfs_root_id(bbio->inode->root),
+ btrfs_ino(bbio->inode), logical, length);
bio_for_each_bvec(bvec, &bbio->bio, iter)
- ASSERT(IS_ALIGNED(bvec.bv_offset, blocksize) &&
- IS_ALIGNED(bvec.bv_len, blocksize),
+ ASSERT(IS_ALIGNED(bvec.bv_offset, alignment) &&
+ IS_ALIGNED(bvec.bv_len, alignment),
"root=%llu inode=%llu logical=%llu length=%u index=%u bv_offset=%u bv_len=%u",
btrfs_root_id(bbio->inode->root),
- btrfs_ino(bbio->inode),
- bbio->bio.bi_iter.bi_sector << SECTOR_SHIFT,
- bbio->bio.bi_iter.bi_size, iter.bi_idx,
- bvec.bv_offset,
- bvec.bv_len);
+ btrfs_ino(bbio->inode), logical, length, iter.bi_idx,
+ bvec.bv_offset, bvec.bv_len);
#endif
}
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 938286bee6a8..552ad99e89fc 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4418,10 +4418,6 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
goto out_acct;
}
- if (fs_info->sectorsize > PAGE_SIZE) {
- ret = -ENOTTY;
- goto out_acct;
- }
if (compat) {
#if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
struct btrfs_ioctl_encoded_io_args_32 args32;
@@ -4513,7 +4509,6 @@ static int btrfs_ioctl_encoded_read(struct file *file, void __user *argp,
static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp, bool compat)
{
- struct btrfs_fs_info *fs_info = inode_to_fs_info(file->f_inode);
struct btrfs_ioctl_encoded_io_args args;
struct iovec iovstack[UIO_FASTIOV];
struct iovec *iov = iovstack;
@@ -4527,11 +4522,6 @@ static int btrfs_ioctl_encoded_write(struct file *file, void __user *argp, bool
goto out_acct;
}
- if (fs_info->sectorsize > PAGE_SIZE) {
- ret = -ENOTTY;
- goto out_acct;
- }
-
if (!(file->f_mode & FMODE_WRITE)) {
ret = -EBADF;
goto out_acct;
@@ -4813,11 +4803,6 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
ret = -EPERM;
goto out_acct;
}
- if (fs_info->sectorsize > PAGE_SIZE) {
- ret = -ENOTTY;
- goto out_acct;
- }
-
sqe_addr = u64_to_user_ptr(READ_ONCE(cmd->sqe->addr));
if (issue_flags & IO_URING_F_COMPAT) {
@@ -4945,7 +4930,6 @@ static int btrfs_uring_encoded_read(struct io_uring_cmd *cmd, unsigned int issue
static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issue_flags)
{
struct file *file = cmd->file;
- struct btrfs_fs_info *fs_info = inode_to_fs_info(file->f_inode);
loff_t pos;
struct kiocb kiocb;
ssize_t ret;
@@ -4960,11 +4944,6 @@ static int btrfs_uring_encoded_write(struct io_uring_cmd *cmd, unsigned int issu
ret = -EPERM;
goto out_acct;
}
- if (fs_info->sectorsize > PAGE_SIZE) {
- ret = -ENOTTY;
- goto out_acct;
- }
-
sqe_addr = u64_to_user_ptr(READ_ONCE(cmd->sqe->addr));
if (!(file->f_mode & FMODE_WRITE)) {
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 9230e5066fc6..1fcc820f6baf 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5601,14 +5601,7 @@ static int send_extent_data(struct send_ctx *sctx, struct btrfs_path *path,
ei = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_file_extent_item);
- /*
- * Do not go through encoded read for bs > ps cases.
- *
- * Encoded send is using vmallocated pages as buffer, which we can
- * not ensure every folio is large enough to contain a block.
- */
- if (sctx->send_root->fs_info->sectorsize <= PAGE_SIZE &&
- (sctx->flags & BTRFS_SEND_FLAG_COMPRESSED) &&
+ if ((sctx->flags & BTRFS_SEND_FLAG_COMPRESSED) &&
btrfs_file_extent_compression(leaf, ei) != BTRFS_COMPRESS_NONE) {
bool is_inline = (btrfs_file_extent_type(leaf, ei) ==
BTRFS_FILE_EXTENT_INLINE);
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread