* [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
@ 2026-09-01 0:01 Qu Wenruo
2026-09-01 5:07 ` Daniel Vacek
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-09-01 0:01 UTC (permalink / raw)
To: linux-btrfs; +Cc: Boris Burkov
In submit_one_sector() we call btrfs_get_extent() to grab the IO extent
map so that we know where the logical location to submit the block.
However there is no guarantee that there is an IO extent map for the
block, and if there is no IO extent map nor ordered extent,
btrfs_get_extent() can grab the file extent from on-disk metadata.
That's why we have ASSERT()s to reject holes and compressed file
extents.
On the other hand, for the write range we should have both an IO extent
map and an ordered extent, so there is no reason not to grab the ordered
extent instead.
There is some minor advantages:
- No hole ordered extent
So no need to rely on ASSERT()s to reject hole extents.
And the ASSERT()s are depending on the kernel config, without
CONFIG_BTRFS_ASSERT those ASSERT()s won't even trigger.
- No IO errors
Unlike btrfs_get_extent() which can return IO error when doing the
metadata tree search, btrfs_lookup_ordered_extent() will either return
an OE or not found.
- Cached OE in bio_ctrl->bbio
At bbio allocation we have already did an OE lookup, and we have a
high chance that the current block also belongs to that OE.
Use that cached OE can reduce the frequency to do an rb-tree search.
- Smaller rb-tree
Unlike extent-map-tree, which can contain cached extent maps, the life
span of ordered extents are much shorter, they get removed from the
ordered tree after the file extent item is inserted into the subvolume
tree.
So doing ordered extent tree search can be a tiny faster.
And since we're here, also address some minor points:
- Add error message for every EUCLEAN error
- Remove a dead comment on btrfs_folio_clear_dirty()
We no longer call folio_clear_dirty_for_io() since commit 095be159f3eb
("btrfs: unify folio dirty flag clearing"), so the folio flag is
still dirty, and the folio dirty flag will be cleared by the last dirty
block.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v3:
- Refactor the helper to grab the OE and increase its refs in one go
v2:
- Extract a helper to determine if the @filepos is in the bbio OE range
Which reduces several "bio_ctrl->bbio->ordered" duplication, and use
in_range() to make is easier to read.
---
fs/btrfs/extent_io.c | 64 +++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d7600e5fa3d9..a221b63bdb20 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1808,6 +1808,22 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode,
return 0;
}
+static struct btrfs_ordered_extent *get_oe_from_bbio(const struct btrfs_bio *bbio,
+ u64 filepos)
+{
+ struct btrfs_ordered_extent *oe;
+
+ if (!bbio || !bbio->ordered)
+ return NULL;
+
+ oe = bbio->ordered;
+ if (!in_range(filepos, oe->file_offset, oe->num_bytes))
+ return NULL;
+
+ refcount_inc(&oe->refs);
+ return oe;
+}
+
/*
* Return 0 if we have submitted or queued the sector for submission.
* Return <0 for critical errors, and the involved sector will be cleaned up.
@@ -1820,11 +1836,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
loff_t i_size)
{
struct btrfs_fs_info *fs_info = inode->root->fs_info;
- struct extent_map *em;
+ struct btrfs_ordered_extent *oe;
u64 block_start;
u64 disk_bytenr;
u64 extent_offset;
- u64 em_end;
const u32 sectorsize = fs_info->sectorsize;
unsigned int queued;
@@ -1833,8 +1848,11 @@ static int submit_one_sector(struct btrfs_inode *inode,
/* @filepos >= i_size case should be handled by the caller. */
ASSERT(filepos < i_size);
- em = btrfs_get_extent(inode, NULL, filepos, sectorsize);
- if (IS_ERR(em)) {
+ /* Try to reuse the existing OE from bbio first. */
+ oe = get_oe_from_bbio(bio_ctrl->bbio, filepos);
+ if (!oe)
+ oe = btrfs_lookup_ordered_extent(inode, filepos);
+ if (unlikely(!oe)) {
/*
* bio_ctrl may contain a bio crossing several folios.
* Submit it immediately so that the bio has a chance
@@ -1857,31 +1875,25 @@ static int submit_one_sector(struct btrfs_inode *inode,
*/
btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
false);
- return PTR_ERR(em);
+ btrfs_err_rl(fs_info,
+ "no ordered extent for root %lld ino %llu filepos %llu",
+ btrfs_root_id(inode->root), btrfs_ino(inode),
+ filepos);
+ return -EUCLEAN;
}
- extent_offset = filepos - em->start;
- em_end = btrfs_extent_map_end(em);
- ASSERT(filepos <= em_end);
- ASSERT(IS_ALIGNED(em->start, sectorsize));
- ASSERT(IS_ALIGNED(em->len, sectorsize));
+ extent_offset = filepos - oe->file_offset;
+ ASSERT(filepos < oe->file_offset + oe->num_bytes);
+ ASSERT(IS_ALIGNED(oe->file_offset, sectorsize));
+ ASSERT(IS_ALIGNED(oe->num_bytes, sectorsize));
+ ASSERT(oe->compress_type == BTRFS_COMPRESS_NONE);
+ ASSERT(!test_bit(BTRFS_ORDERED_COMPRESSED, &oe->flags));
- block_start = btrfs_extent_map_block_start(em);
- disk_bytenr = btrfs_extent_map_block_start(em) + extent_offset;
+ block_start = oe->disk_bytenr + oe->offset;
+ disk_bytenr = block_start + extent_offset;
- ASSERT(!btrfs_extent_map_is_compressed(em));
- ASSERT(block_start != EXTENT_MAP_HOLE);
- ASSERT(block_start != EXTENT_MAP_INLINE);
+ btrfs_put_ordered_extent(oe);
- btrfs_free_extent_map(em);
- em = NULL;
-
- /*
- * Although the PageDirty bit is cleared before entering this
- * function, subpage dirty bit is not cleared.
- * So clear subpage dirty bit here so next time we won't submit
- * a folio for a range already written to disk.
- */
btrfs_folio_clear_dirty(fs_info, folio, filepos, sectorsize);
btrfs_folio_set_writeback(fs_info, folio, filepos, sectorsize);
/*
@@ -1898,6 +1910,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
btrfs_folio_clear_writeback(fs_info, folio, filepos, sectorsize);
btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
false);
+ btrfs_err_rl(fs_info,
+ "failed to queue sector for root %lld ino %llu filepos %llu",
+ btrfs_root_id(inode->root),
+ btrfs_ino(inode), filepos);
return -EUCLEAN;
}
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
2026-09-01 0:01 [PATCH v3] btrfs: use ordered extent to grab the logical address for submission Qu Wenruo
@ 2026-09-01 5:07 ` Daniel Vacek
2026-09-01 7:11 ` Johannes Thumshirn
2026-09-02 18:23 ` Daniel Vacek
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Vacek @ 2026-09-01 5:07 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Boris Burkov
On Tue, 1 Sept 2026 at 02:02, Qu Wenruo <wqu@suse.com> wrote:
> In submit_one_sector() we call btrfs_get_extent() to grab the IO extent
> map so that we know where the logical location to submit the block.
>
> However there is no guarantee that there is an IO extent map for the
> block, and if there is no IO extent map nor ordered extent,
> btrfs_get_extent() can grab the file extent from on-disk metadata.
>
> That's why we have ASSERT()s to reject holes and compressed file
> extents.
>
> On the other hand, for the write range we should have both an IO extent
> map and an ordered extent, so there is no reason not to grab the ordered
> extent instead.
>
> There is some minor advantages:
>
> - No hole ordered extent
> So no need to rely on ASSERT()s to reject hole extents.
>
> And the ASSERT()s are depending on the kernel config, without
> CONFIG_BTRFS_ASSERT those ASSERT()s won't even trigger.
>
> - No IO errors
> Unlike btrfs_get_extent() which can return IO error when doing the
> metadata tree search, btrfs_lookup_ordered_extent() will either return
> an OE or not found.
>
> - Cached OE in bio_ctrl->bbio
> At bbio allocation we have already did an OE lookup, and we have a
> high chance that the current block also belongs to that OE.
> Use that cached OE can reduce the frequency to do an rb-tree search.
>
> - Smaller rb-tree
> Unlike extent-map-tree, which can contain cached extent maps, the life
> span of ordered extents are much shorter, they get removed from the
> ordered tree after the file extent item is inserted into the subvolume
> tree.
>
> So doing ordered extent tree search can be a tiny faster.
>
> And since we're here, also address some minor points:
>
> - Add error message for every EUCLEAN error
>
> - Remove a dead comment on btrfs_folio_clear_dirty()
> We no longer call folio_clear_dirty_for_io() since commit 095be159f3eb
> ("btrfs: unify folio dirty flag clearing"), so the folio flag is
> still dirty, and the folio dirty flag will be cleared by the last dirty
> block.
>
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
LGTM
Reviewed-by: Daniel Vacek <neelx@suse.com>
> ---
> Changelog:
> v3:
> - Refactor the helper to grab the OE and increase its refs in one go
>
> v2:
> - Extract a helper to determine if the @filepos is in the bbio OE range
> Which reduces several "bio_ctrl->bbio->ordered" duplication, and use
> in_range() to make is easier to read.
> ---
> fs/btrfs/extent_io.c | 64 +++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index d7600e5fa3d9..a221b63bdb20 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1808,6 +1808,22 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode,
> return 0;
> }
>
> +static struct btrfs_ordered_extent *get_oe_from_bbio(const struct btrfs_bio *bbio,
> + u64 filepos)
> +{
> + struct btrfs_ordered_extent *oe;
> +
> + if (!bbio || !bbio->ordered)
> + return NULL;
> +
> + oe = bbio->ordered;
> + if (!in_range(filepos, oe->file_offset, oe->num_bytes))
> + return NULL;
> +
> + refcount_inc(&oe->refs);
> + return oe;
> +}
> +
> /*
> * Return 0 if we have submitted or queued the sector for submission.
> * Return <0 for critical errors, and the involved sector will be cleaned up.
> @@ -1820,11 +1836,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
> loff_t i_size)
> {
> struct btrfs_fs_info *fs_info = inode->root->fs_info;
> - struct extent_map *em;
> + struct btrfs_ordered_extent *oe;
> u64 block_start;
> u64 disk_bytenr;
> u64 extent_offset;
> - u64 em_end;
> const u32 sectorsize = fs_info->sectorsize;
> unsigned int queued;
>
> @@ -1833,8 +1848,11 @@ static int submit_one_sector(struct btrfs_inode *inode,
> /* @filepos >= i_size case should be handled by the caller. */
> ASSERT(filepos < i_size);
>
> - em = btrfs_get_extent(inode, NULL, filepos, sectorsize);
> - if (IS_ERR(em)) {
> + /* Try to reuse the existing OE from bbio first. */
> + oe = get_oe_from_bbio(bio_ctrl->bbio, filepos);
> + if (!oe)
> + oe = btrfs_lookup_ordered_extent(inode, filepos);
> + if (unlikely(!oe)) {
> /*
> * bio_ctrl may contain a bio crossing several folios.
> * Submit it immediately so that the bio has a chance
> @@ -1857,31 +1875,25 @@ static int submit_one_sector(struct btrfs_inode *inode,
> */
> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
> false);
> - return PTR_ERR(em);
> + btrfs_err_rl(fs_info,
> + "no ordered extent for root %lld ino %llu filepos %llu",
> + btrfs_root_id(inode->root), btrfs_ino(inode),
> + filepos);
> + return -EUCLEAN;
> }
>
> - extent_offset = filepos - em->start;
> - em_end = btrfs_extent_map_end(em);
> - ASSERT(filepos <= em_end);
> - ASSERT(IS_ALIGNED(em->start, sectorsize));
> - ASSERT(IS_ALIGNED(em->len, sectorsize));
> + extent_offset = filepos - oe->file_offset;
> + ASSERT(filepos < oe->file_offset + oe->num_bytes);
> + ASSERT(IS_ALIGNED(oe->file_offset, sectorsize));
> + ASSERT(IS_ALIGNED(oe->num_bytes, sectorsize));
> + ASSERT(oe->compress_type == BTRFS_COMPRESS_NONE);
> + ASSERT(!test_bit(BTRFS_ORDERED_COMPRESSED, &oe->flags));
>
> - block_start = btrfs_extent_map_block_start(em);
> - disk_bytenr = btrfs_extent_map_block_start(em) + extent_offset;
> + block_start = oe->disk_bytenr + oe->offset;
> + disk_bytenr = block_start + extent_offset;
>
> - ASSERT(!btrfs_extent_map_is_compressed(em));
> - ASSERT(block_start != EXTENT_MAP_HOLE);
> - ASSERT(block_start != EXTENT_MAP_INLINE);
> + btrfs_put_ordered_extent(oe);
>
> - btrfs_free_extent_map(em);
> - em = NULL;
> -
> - /*
> - * Although the PageDirty bit is cleared before entering this
> - * function, subpage dirty bit is not cleared.
> - * So clear subpage dirty bit here so next time we won't submit
> - * a folio for a range already written to disk.
> - */
> btrfs_folio_clear_dirty(fs_info, folio, filepos, sectorsize);
> btrfs_folio_set_writeback(fs_info, folio, filepos, sectorsize);
> /*
> @@ -1898,6 +1910,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
> btrfs_folio_clear_writeback(fs_info, folio, filepos, sectorsize);
> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
> false);
> + btrfs_err_rl(fs_info,
> + "failed to queue sector for root %lld ino %llu filepos %llu",
> + btrfs_root_id(inode->root),
> + btrfs_ino(inode), filepos);
> return -EUCLEAN;
> }
> return 0;
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
2026-09-01 0:01 [PATCH v3] btrfs: use ordered extent to grab the logical address for submission Qu Wenruo
2026-09-01 5:07 ` Daniel Vacek
@ 2026-09-01 7:11 ` Johannes Thumshirn
2026-09-02 18:23 ` Daniel Vacek
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2026-09-01 7:11 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Boris Burkov
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
2026-09-01 0:01 [PATCH v3] btrfs: use ordered extent to grab the logical address for submission Qu Wenruo
2026-09-01 5:07 ` Daniel Vacek
2026-09-01 7:11 ` Johannes Thumshirn
@ 2026-09-02 18:23 ` Daniel Vacek
2026-09-02 21:44 ` Qu Wenruo
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Vacek @ 2026-09-02 18:23 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, Boris Burkov
On Tue, 1 Sept 2026 at 02:02, Qu Wenruo <wqu@suse.com> wrote:
> In submit_one_sector() we call btrfs_get_extent() to grab the IO extent
> map so that we know where the logical location to submit the block.
>
> However there is no guarantee that there is an IO extent map for the
> block, and if there is no IO extent map nor ordered extent,
> btrfs_get_extent() can grab the file extent from on-disk metadata.
>
> That's why we have ASSERT()s to reject holes and compressed file
> extents.
>
> On the other hand, for the write range we should have both an IO extent
> map and an ordered extent, so there is no reason not to grab the ordered
> extent instead.
>
> There is some minor advantages:
>
> - No hole ordered extent
> So no need to rely on ASSERT()s to reject hole extents.
>
> And the ASSERT()s are depending on the kernel config, without
> CONFIG_BTRFS_ASSERT those ASSERT()s won't even trigger.
>
> - No IO errors
> Unlike btrfs_get_extent() which can return IO error when doing the
> metadata tree search, btrfs_lookup_ordered_extent() will either return
> an OE or not found.
>
> - Cached OE in bio_ctrl->bbio
> At bbio allocation we have already did an OE lookup, and we have a
> high chance that the current block also belongs to that OE.
> Use that cached OE can reduce the frequency to do an rb-tree search.
>
> - Smaller rb-tree
> Unlike extent-map-tree, which can contain cached extent maps, the life
> span of ordered extents are much shorter, they get removed from the
> ordered tree after the file extent item is inserted into the subvolume
> tree.
>
> So doing ordered extent tree search can be a tiny faster.
>
> And since we're here, also address some minor points:
>
> - Add error message for every EUCLEAN error
>
> - Remove a dead comment on btrfs_folio_clear_dirty()
> We no longer call folio_clear_dirty_for_io() since commit 095be159f3eb
> ("btrfs: unify folio dirty flag clearing"), so the folio flag is
> still dirty, and the folio dirty flag will be cleared by the last dirty
> block.
>
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> ---
> Changelog:
> v3:
> - Refactor the helper to grab the OE and increase its refs in one go
>
> v2:
> - Extract a helper to determine if the @filepos is in the bbio OE range
> Which reduces several "bio_ctrl->bbio->ordered" duplication, and use
> in_range() to make is easier to read.
> ---
> fs/btrfs/extent_io.c | 64 +++++++++++++++++++++++++++-----------------
> 1 file changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index d7600e5fa3d9..a221b63bdb20 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1808,6 +1808,22 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode,
> return 0;
> }
>
> +static struct btrfs_ordered_extent *get_oe_from_bbio(const struct btrfs_bio *bbio,
> + u64 filepos)
> +{
> + struct btrfs_ordered_extent *oe;
> +
> + if (!bbio || !bbio->ordered)
> + return NULL;
> +
> + oe = bbio->ordered;
> + if (!in_range(filepos, oe->file_offset, oe->num_bytes))
> + return NULL;
> +
> + refcount_inc(&oe->refs);
Thinking about it again, do we even need to raise the refcount?
One reference is already held by the bbio itself, so the lifetime is guaranteed.
> + return oe;
> +}
> +
> /*
> * Return 0 if we have submitted or queued the sector for submission.
> * Return <0 for critical errors, and the involved sector will be cleaned up.
> @@ -1820,11 +1836,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
> loff_t i_size)
> {
> struct btrfs_fs_info *fs_info = inode->root->fs_info;
> - struct extent_map *em;
> + struct btrfs_ordered_extent *oe;
> u64 block_start;
> u64 disk_bytenr;
> u64 extent_offset;
> - u64 em_end;
> const u32 sectorsize = fs_info->sectorsize;
> unsigned int queued;
>
> @@ -1833,8 +1848,11 @@ static int submit_one_sector(struct btrfs_inode *inode,
> /* @filepos >= i_size case should be handled by the caller. */
> ASSERT(filepos < i_size);
>
> - em = btrfs_get_extent(inode, NULL, filepos, sectorsize);
> - if (IS_ERR(em)) {
> + /* Try to reuse the existing OE from bbio first. */
> + oe = get_oe_from_bbio(bio_ctrl->bbio, filepos);
> + if (!oe)
{
> + oe = btrfs_lookup_ordered_extent(inode, filepos);
put = true; // or !!oe
}
> + if (unlikely(!oe)) {
> /*
> * bio_ctrl may contain a bio crossing several folios.
> * Submit it immediately so that the bio has a chance
> @@ -1857,31 +1875,25 @@ static int submit_one_sector(struct btrfs_inode *inode,
> */
> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
> false);
> - return PTR_ERR(em);
> + btrfs_err_rl(fs_info,
> + "no ordered extent for root %lld ino %llu filepos %llu",
> + btrfs_root_id(inode->root), btrfs_ino(inode),
> + filepos);
> + return -EUCLEAN;
> }
>
> - extent_offset = filepos - em->start;
> - em_end = btrfs_extent_map_end(em);
> - ASSERT(filepos <= em_end);
> - ASSERT(IS_ALIGNED(em->start, sectorsize));
> - ASSERT(IS_ALIGNED(em->len, sectorsize));
> + extent_offset = filepos - oe->file_offset;
> + ASSERT(filepos < oe->file_offset + oe->num_bytes);
> + ASSERT(IS_ALIGNED(oe->file_offset, sectorsize));
> + ASSERT(IS_ALIGNED(oe->num_bytes, sectorsize));
> + ASSERT(oe->compress_type == BTRFS_COMPRESS_NONE);
> + ASSERT(!test_bit(BTRFS_ORDERED_COMPRESSED, &oe->flags));
>
> - block_start = btrfs_extent_map_block_start(em);
> - disk_bytenr = btrfs_extent_map_block_start(em) + extent_offset;
> + block_start = oe->disk_bytenr + oe->offset;
> + disk_bytenr = block_start + extent_offset;
>
> - ASSERT(!btrfs_extent_map_is_compressed(em));
> - ASSERT(block_start != EXTENT_MAP_HOLE);
> - ASSERT(block_start != EXTENT_MAP_INLINE);
if (put)
> + btrfs_put_ordered_extent(oe);
>
> - btrfs_free_extent_map(em);
> - em = NULL;
> -
> - /*
> - * Although the PageDirty bit is cleared before entering this
> - * function, subpage dirty bit is not cleared.
> - * So clear subpage dirty bit here so next time we won't submit
> - * a folio for a range already written to disk.
> - */
> btrfs_folio_clear_dirty(fs_info, folio, filepos, sectorsize);
> btrfs_folio_set_writeback(fs_info, folio, filepos, sectorsize);
> /*
> @@ -1898,6 +1910,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
> btrfs_folio_clear_writeback(fs_info, folio, filepos, sectorsize);
> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
> false);
> + btrfs_err_rl(fs_info,
> + "failed to queue sector for root %lld ino %llu filepos %llu",
> + btrfs_root_id(inode->root),
> + btrfs_ino(inode), filepos);
> return -EUCLEAN;
> }
> return 0;
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
2026-09-02 18:23 ` Daniel Vacek
@ 2026-09-02 21:44 ` Qu Wenruo
2026-09-03 6:30 ` Daniel Vacek
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2026-09-02 21:44 UTC (permalink / raw)
To: Daniel Vacek, Qu Wenruo; +Cc: linux-btrfs, Boris Burkov
在 2026/9/3 03:53, Daniel Vacek 写道:
> On Tue, 1 Sept 2026 at 02:02, Qu Wenruo <wqu@suse.com> wrote:
>> In submit_one_sector() we call btrfs_get_extent() to grab the IO extent
>> map so that we know where the logical location to submit the block.
>>
>> However there is no guarantee that there is an IO extent map for the
>> block, and if there is no IO extent map nor ordered extent,
>> btrfs_get_extent() can grab the file extent from on-disk metadata.
>>
>> That's why we have ASSERT()s to reject holes and compressed file
>> extents.
>>
>> On the other hand, for the write range we should have both an IO extent
>> map and an ordered extent, so there is no reason not to grab the ordered
>> extent instead.
>>
>> There is some minor advantages:
>>
>> - No hole ordered extent
>> So no need to rely on ASSERT()s to reject hole extents.
>>
>> And the ASSERT()s are depending on the kernel config, without
>> CONFIG_BTRFS_ASSERT those ASSERT()s won't even trigger.
>>
>> - No IO errors
>> Unlike btrfs_get_extent() which can return IO error when doing the
>> metadata tree search, btrfs_lookup_ordered_extent() will either return
>> an OE or not found.
>>
>> - Cached OE in bio_ctrl->bbio
>> At bbio allocation we have already did an OE lookup, and we have a
>> high chance that the current block also belongs to that OE.
>> Use that cached OE can reduce the frequency to do an rb-tree search.
>>
>> - Smaller rb-tree
>> Unlike extent-map-tree, which can contain cached extent maps, the life
>> span of ordered extents are much shorter, they get removed from the
>> ordered tree after the file extent item is inserted into the subvolume
>> tree.
>>
>> So doing ordered extent tree search can be a tiny faster.
>>
>> And since we're here, also address some minor points:
>>
>> - Add error message for every EUCLEAN error
>>
>> - Remove a dead comment on btrfs_folio_clear_dirty()
>> We no longer call folio_clear_dirty_for_io() since commit 095be159f3eb
>> ("btrfs: unify folio dirty flag clearing"), so the folio flag is
>> still dirty, and the folio dirty flag will be cleared by the last dirty
>> block.
>>
>> Reviewed-by: Boris Burkov <boris@bur.io>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>
>> ---
>> Changelog:
>> v3:
>> - Refactor the helper to grab the OE and increase its refs in one go
>>
>> v2:
>> - Extract a helper to determine if the @filepos is in the bbio OE range
>> Which reduces several "bio_ctrl->bbio->ordered" duplication, and use
>> in_range() to make is easier to read.
>> ---
>> fs/btrfs/extent_io.c | 64 +++++++++++++++++++++++++++-----------------
>> 1 file changed, 40 insertions(+), 24 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index d7600e5fa3d9..a221b63bdb20 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -1808,6 +1808,22 @@ static noinline_for_stack int writepage_delalloc(struct btrfs_inode *inode,
>> return 0;
>> }
>>
>> +static struct btrfs_ordered_extent *get_oe_from_bbio(const struct btrfs_bio *bbio,
>> + u64 filepos)
>> +{
>> + struct btrfs_ordered_extent *oe;
>> +
>> + if (!bbio || !bbio->ordered)
>> + return NULL;
>> +
>> + oe = bbio->ordered;
>> + if (!in_range(filepos, oe->file_offset, oe->num_bytes))
>> + return NULL;
>> +
>> + refcount_inc(&oe->refs);
>
> Thinking about it again, do we even need to raise the refcount?
That is to co-operate with that btrfs_put_ordered_extent().
>
> One reference is already held by the bbio itself, so the lifetime is guaranteed.
>
>> + return oe;
>> +}
>> +
>> /*
>> * Return 0 if we have submitted or queued the sector for submission.
>> * Return <0 for critical errors, and the involved sector will be cleaned up.
>> @@ -1820,11 +1836,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
>> loff_t i_size)
>> {
>> struct btrfs_fs_info *fs_info = inode->root->fs_info;
>> - struct extent_map *em;
>> + struct btrfs_ordered_extent *oe;
>> u64 block_start;
>> u64 disk_bytenr;
>> u64 extent_offset;
>> - u64 em_end;
>> const u32 sectorsize = fs_info->sectorsize;
>> unsigned int queued;
>>
>> @@ -1833,8 +1848,11 @@ static int submit_one_sector(struct btrfs_inode *inode,
>> /* @filepos >= i_size case should be handled by the caller. */
>> ASSERT(filepos < i_size);
>>
>> - em = btrfs_get_extent(inode, NULL, filepos, sectorsize);
>> - if (IS_ERR(em)) {
>> + /* Try to reuse the existing OE from bbio first. */
>> + oe = get_oe_from_bbio(bio_ctrl->bbio, filepos);
>> + if (!oe)
> {
>> + oe = btrfs_lookup_ordered_extent(inode, filepos);
> put = true; // or !!oe
I prefer not to introduce an extra if check just for a single case.
So the existing unconditional put and refcount inc for the OE from bbio
looks fine to me.
Thanks,
Qu
> }
>> + if (unlikely(!oe)) {
>> /*
>> * bio_ctrl may contain a bio crossing several folios.
>> * Submit it immediately so that the bio has a chance
>> @@ -1857,31 +1875,25 @@ static int submit_one_sector(struct btrfs_inode *inode,
>> */
>> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
>> false);
>> - return PTR_ERR(em);
>> + btrfs_err_rl(fs_info,
>> + "no ordered extent for root %lld ino %llu filepos %llu",
>> + btrfs_root_id(inode->root), btrfs_ino(inode),
>> + filepos);
>> + return -EUCLEAN;
>> }
>>
>> - extent_offset = filepos - em->start;
>> - em_end = btrfs_extent_map_end(em);
>> - ASSERT(filepos <= em_end);
>> - ASSERT(IS_ALIGNED(em->start, sectorsize));
>> - ASSERT(IS_ALIGNED(em->len, sectorsize));
>> + extent_offset = filepos - oe->file_offset;
>> + ASSERT(filepos < oe->file_offset + oe->num_bytes);
>> + ASSERT(IS_ALIGNED(oe->file_offset, sectorsize));
>> + ASSERT(IS_ALIGNED(oe->num_bytes, sectorsize));
>> + ASSERT(oe->compress_type == BTRFS_COMPRESS_NONE);
>> + ASSERT(!test_bit(BTRFS_ORDERED_COMPRESSED, &oe->flags));
>>
>> - block_start = btrfs_extent_map_block_start(em);
>> - disk_bytenr = btrfs_extent_map_block_start(em) + extent_offset;
>> + block_start = oe->disk_bytenr + oe->offset;
>> + disk_bytenr = block_start + extent_offset;
>>
>> - ASSERT(!btrfs_extent_map_is_compressed(em));
>> - ASSERT(block_start != EXTENT_MAP_HOLE);
>> - ASSERT(block_start != EXTENT_MAP_INLINE);
>
> if (put)
>
>> + btrfs_put_ordered_extent(oe);
>>
>> - btrfs_free_extent_map(em);
>> - em = NULL;
>> -
>> - /*
>> - * Although the PageDirty bit is cleared before entering this
>> - * function, subpage dirty bit is not cleared.
>> - * So clear subpage dirty bit here so next time we won't submit
>> - * a folio for a range already written to disk.
>> - */
>> btrfs_folio_clear_dirty(fs_info, folio, filepos, sectorsize);
>> btrfs_folio_set_writeback(fs_info, folio, filepos, sectorsize);
>> /*
>> @@ -1898,6 +1910,10 @@ static int submit_one_sector(struct btrfs_inode *inode,
>> btrfs_folio_clear_writeback(fs_info, folio, filepos, sectorsize);
>> btrfs_mark_ordered_io_finished(inode, filepos, fs_info->sectorsize,
>> false);
>> + btrfs_err_rl(fs_info,
>> + "failed to queue sector for root %lld ino %llu filepos %llu",
>> + btrfs_root_id(inode->root),
>> + btrfs_ino(inode), filepos);
>> return -EUCLEAN;
>> }
>> return 0;
>> --
>> 2.55.0
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] btrfs: use ordered extent to grab the logical address for submission
2026-09-02 21:44 ` Qu Wenruo
@ 2026-09-03 6:30 ` Daniel Vacek
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vacek @ 2026-09-03 6:30 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, Boris Burkov
On Wed, 2 Sept 2026 at 23:44, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2026/9/3 03:53, Daniel Vacek 写道:
> > On Tue, 1 Sept 2026 at 02:02, Qu Wenruo <wqu@suse.com> wrote:
> >> + if (!in_range(filepos, oe->file_offset, oe->num_bytes))
> >> + return NULL;
> >> +
> >> + refcount_inc(&oe->refs);
> >
> > Thinking about it again, do we even need to raise the refcount?
>
> That is to co-operate with that btrfs_put_ordered_extent().
>
> > One reference is already held by the bbio itself, so the lifetime is guaranteed.
> >
> >> + return oe;
> >> +}
> >> +
> >> /*
> >> * Return 0 if we have submitted or queued the sector for submission.
> >> * Return <0 for critical errors, and the involved sector will be cleaned up.
> >> @@ -1833,8 +1848,11 @@ static int submit_one_sector(struct btrfs_inode *inode,
> >> /* @filepos >= i_size case should be handled by the caller. */
> >> ASSERT(filepos < i_size);
> >>
> >> - em = btrfs_get_extent(inode, NULL, filepos, sectorsize);
> >> - if (IS_ERR(em)) {
> >> + /* Try to reuse the existing OE from bbio first. */
> >> + oe = get_oe_from_bbio(bio_ctrl->bbio, filepos);
> >> + if (!oe)
> > {
> >> + oe = btrfs_lookup_ordered_extent(inode, filepos);
> > put = true; // or !!oe
>
> I prefer not to introduce an extra if check just for a single case.
>
> So the existing unconditional put and refcount inc for the OE from bbio
> looks fine to me.
Fair enough.
--nX
> Thanks,
> Qu
>
> >> - ASSERT(!btrfs_extent_map_is_compressed(em));
> >> - ASSERT(block_start != EXTENT_MAP_HOLE);
> >> - ASSERT(block_start != EXTENT_MAP_INLINE);
> >
> > if (put)
> >
> >> + btrfs_put_ordered_extent(oe);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-03 6:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 0:01 [PATCH v3] btrfs: use ordered extent to grab the logical address for submission Qu Wenruo
2026-09-01 5:07 ` Daniel Vacek
2026-09-01 7:11 ` Johannes Thumshirn
2026-09-02 18:23 ` Daniel Vacek
2026-09-02 21:44 ` Qu Wenruo
2026-09-03 6:30 ` Daniel Vacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox