* [PATCH 0/3] btrfs: simple enhancement and fix for delalloc range
@ 2025-07-20 6:29 Qu Wenruo
2025-07-20 6:29 ` [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range() Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Qu Wenruo @ 2025-07-20 6:29 UTC (permalink / raw)
To: linux-btrfs
When debugging the rare DEBUG_WARN() inside btrfs_writepage_cow_fixup(),
I just found several small things to fix/enhance:
- The double boolean parameters of cow_file_range()
A simple enhancement with a single @flags to replace the two booleans.
- Inefficient folio iteration of btrfs_cleanup_ordered_extents()
Enhance that function to support large folios better.
- Wrong parameters for btrfs_cleanup_ordered_extents() of
nocow_one_range()
Due to the wrong parameter (end other than length), we may clear
Ordered flags for range out of our control, which can be some page
under writeback and cause the DEBUG_WARN() inside
btrfs_writepage_cow_fixup().
However the call trace doesn't match the one I hit, which is normally
cow_file_range() failure inside fallback_to_cow().
Not the failure from nocow_one_range().
Although I already have an idea why the DEBUG_WARN() in
btrfs_writepage_cow_fixup() is triggered, the fix will be a little more
complex.
The root cause is that btrfs_cleanup_ordered_extents() is called on
unlocked folios inside run_delalloc_nocow().
As run_delalloc_nocow() can fallback to COW path, and succeeded
cow_file_range() will unlock the folios, we have no other way but to do
the cleanup on unlocked folios.
This has a small race window, where the unlocked folio is already under
writeback, and we cleared the ordered flag halfway, thus triggerring the
error.
The proper fix is to make run_delalloc_nocow() to keep folios
(and io tree range) locked until the full range is finished, just like
how cow_file_range() works, but that will be a much more complex series.
So for now just handle the small-and-simple enhancement and fix.
Qu Wenruo (3):
btrfs: replace double boolean parameters of cow_file_range()
btrfs: make btrfs_cleanup_ordered_extents() to support large folios
btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents()
fs/btrfs/inode.c | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
--
2.50.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range()
2025-07-20 6:29 [PATCH 0/3] btrfs: simple enhancement and fix for delalloc range Qu Wenruo
@ 2025-07-20 6:29 ` Qu Wenruo
2025-07-21 20:29 ` Boris Burkov
2025-07-20 6:29 ` [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios Qu Wenruo
2025-07-20 6:29 ` [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents() Qu Wenruo
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2025-07-20 6:29 UTC (permalink / raw)
To: linux-btrfs
The function cow_file_range() has two boolean parameters, which is never
a good thing for eyes.
Replace it with a single @flags parameter, with two flags:
- COW_FILE_RANGE_NO_INLINE
- COW_FILE_RANGE_KEEP_LOCKED
And since we're here, also update the comments of cow_file_range() to
replace the old "page" usage with "folio".
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/inode.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b77dd22b8cdb..fc47e234b729 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -72,6 +72,9 @@
#include "raid-stripe-tree.h"
#include "fiemap.h"
+#define COW_FILE_RANGE_KEEP_LOCKED (1UL << 0)
+#define COW_FILE_RANGE_NO_INLINE (1UL << 1)
+
struct btrfs_iget_args {
u64 ino;
struct btrfs_root *root;
@@ -1243,18 +1246,18 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
* locked_folio is the folio that writepage had locked already. We use
* it to make sure we don't do extra locks or unlocks.
*
- * When this function fails, it unlocks all pages except @locked_folio.
+ * When this function fails, it unlocks all folios except @locked_folio.
*
* When this function successfully creates an inline extent, it returns 1 and
- * unlocks all pages including locked_folio and starts I/O on them.
- * (In reality inline extents are limited to a single page, so locked_folio is
- * the only page handled anyway).
+ * unlocks all folios including locked_folio and starts I/O on them.
+ * (In reality inline extents are limited to a single block, so locked_folio is
+ * the only folio handled anyway).
*
- * When this function succeed and creates a normal extent, the page locking
+ * When this function succeed and creates a normal extent, the folio locking
* status depends on the passed in flags:
*
- * - If @keep_locked is set, all pages are kept locked.
- * - Else all pages except for @locked_folio are unlocked.
+ * - If COW_FILE_RANGE_KEEP_LOCKED flag is set, all folios are kept locked.
+ * - Else all folios except for @locked_folio are unlocked.
*
* When a failure happens in the second or later iteration of the
* while-loop, the ordered extents created in previous iterations are cleaned up.
@@ -1262,7 +1265,7 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
static noinline int cow_file_range(struct btrfs_inode *inode,
struct folio *locked_folio, u64 start,
u64 end, u64 *done_offset,
- bool keep_locked, bool no_inline)
+ unsigned long flags)
{
struct btrfs_root *root = inode->root;
struct btrfs_fs_info *fs_info = root->fs_info;
@@ -1290,7 +1293,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
inode_should_defrag(inode, start, end, num_bytes, SZ_64K);
- if (!no_inline) {
+ if (!(flags & COW_FILE_RANGE_NO_INLINE)) {
/* lets try to make an inline extent */
ret = cow_file_range_inline(inode, locked_folio, start, end, 0,
BTRFS_COMPRESS_NONE, NULL, false);
@@ -1318,7 +1321,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
* Do set the Ordered (Private2) bit so we know this page was properly
* setup for writepage.
*/
- page_ops = (keep_locked ? 0 : PAGE_UNLOCK);
+ page_ops = ((flags & COW_FILE_RANGE_KEEP_LOCKED) ? 0 : PAGE_UNLOCK);
page_ops |= PAGE_SET_ORDERED;
/*
@@ -1685,7 +1688,7 @@ static noinline int run_delalloc_cow(struct btrfs_inode *inode,
while (start <= end) {
ret = cow_file_range(inode, locked_folio, start, end,
- &done_offset, true, false);
+ &done_offset, COW_FILE_RANGE_KEEP_LOCKED);
if (ret)
return ret;
extent_write_locked_range(&inode->vfs_inode, locked_folio,
@@ -1767,8 +1770,8 @@ static int fallback_to_cow(struct btrfs_inode *inode,
* is written out and unlocked directly and a normal NOCOW extent
* doesn't work.
*/
- ret = cow_file_range(inode, locked_folio, start, end, NULL, false,
- true);
+ ret = cow_file_range(inode, locked_folio, start, end, NULL,
+ COW_FILE_RANGE_NO_INLINE);
ASSERT(ret != 1);
return ret;
}
@@ -2347,8 +2350,7 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct folio *locked_fol
ret = run_delalloc_cow(inode, locked_folio, start, end, wbc,
true);
else
- ret = cow_file_range(inode, locked_folio, start, end, NULL,
- false, false);
+ ret = cow_file_range(inode, locked_folio, start, end, NULL, 0);
return ret;
}
--
2.50.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios
2025-07-20 6:29 [PATCH 0/3] btrfs: simple enhancement and fix for delalloc range Qu Wenruo
2025-07-20 6:29 ` [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range() Qu Wenruo
@ 2025-07-20 6:29 ` Qu Wenruo
2025-07-21 20:32 ` Boris Burkov
2025-07-20 6:29 ` [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents() Qu Wenruo
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2025-07-20 6:29 UTC (permalink / raw)
To: linux-btrfs
When hitting a large folio, btrfs_cleanup_ordered_extents() will get the
same large folio multiple times, and clearing the same range again and
again.
Thankfully this is not causing anything wrong, just inefficiency.
This is caused by the fact that we're iterating folios using the old
page index, thus can hit the same large folio again and again.
Enhance it by increasing @index to the index of the folio end, and only
increase @index by 1 if we failed to grab a folio.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/inode.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index fc47e234b729..5259bb8ec430 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -404,10 +404,12 @@ static inline void btrfs_cleanup_ordered_extents(struct btrfs_inode *inode,
while (index <= end_index) {
folio = filemap_get_folio(inode->vfs_inode.i_mapping, index);
- index++;
- if (IS_ERR(folio))
+ if (IS_ERR(folio)) {
+ index++;
continue;
+ }
+ index = folio_end(folio) >> PAGE_SHIFT;
/*
* Here we just clear all Ordered bits for every page in the
* range, then btrfs_mark_ordered_io_finished() will handle
--
2.50.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents()
2025-07-20 6:29 [PATCH 0/3] btrfs: simple enhancement and fix for delalloc range Qu Wenruo
2025-07-20 6:29 ` [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range() Qu Wenruo
2025-07-20 6:29 ` [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios Qu Wenruo
@ 2025-07-20 6:29 ` Qu Wenruo
2025-07-21 20:36 ` Boris Burkov
2 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2025-07-20 6:29 UTC (permalink / raw)
To: linux-btrfs
Inside nocow_one_range(), if the checksum cloning for data reloc inode
failed, we call btrfs_cleanup_ordered_extents() to cleanup the just
allocated ordered extents.
But unlike extent_clear_unlock_delalloc(),
btrfs_cleanup_ordered_extents() requires a length, not an inclusive end
bytenr.
This can be problematic, as the @end is normally way larger than @len.
This means btrfs_cleanup_ordered_extents() can be called on folios
out of the correct range, and if the out-of-range folio is under
writeback, we can incorrectly clear the ordered flag of the folio, and
trigger the DEBUG_WARN() inside btrfs_writepage_cow_fixup().
Fix the wrong parameter with correct length instead.
Fixes: 94f6c5c17e52 ("btrfs: move ordered extent cleanup to where they are allocated")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5259bb8ec430..6d9a8d8bea4c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2018,7 +2018,7 @@ static int nocow_one_range(struct btrfs_inode *inode, struct folio *locked_folio
* cleaered by the caller.
*/
if (ret < 0)
- btrfs_cleanup_ordered_extents(inode, file_pos, end);
+ btrfs_cleanup_ordered_extents(inode, file_pos, len);
return ret;
}
--
2.50.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range()
2025-07-20 6:29 ` [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range() Qu Wenruo
@ 2025-07-21 20:29 ` Boris Burkov
2025-07-21 22:59 ` Qu Wenruo
0 siblings, 1 reply; 8+ messages in thread
From: Boris Burkov @ 2025-07-21 20:29 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Sun, Jul 20, 2025 at 03:59:09PM +0930, Qu Wenruo wrote:
> The function cow_file_range() has two boolean parameters, which is never
> a good thing for eyes.
>
> Replace it with a single @flags parameter, with two flags:
>
> - COW_FILE_RANGE_NO_INLINE
> - COW_FILE_RANGE_KEEP_LOCKED
>
> And since we're here, also update the comments of cow_file_range() to
> replace the old "page" usage with "folio".
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Had a nit, but looks good overall. I think the flag approach is nice.
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
> fs/btrfs/inode.c | 32 +++++++++++++++++---------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b77dd22b8cdb..fc47e234b729 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -72,6 +72,9 @@
> #include "raid-stripe-tree.h"
> #include "fiemap.h"
>
> +#define COW_FILE_RANGE_KEEP_LOCKED (1UL << 0)
> +#define COW_FILE_RANGE_NO_INLINE (1UL << 1)
> +
> struct btrfs_iget_args {
> u64 ino;
> struct btrfs_root *root;
> @@ -1243,18 +1246,18 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
> * locked_folio is the folio that writepage had locked already. We use
> * it to make sure we don't do extra locks or unlocks.
> *
> - * When this function fails, it unlocks all pages except @locked_folio.
> + * When this function fails, it unlocks all folios except @locked_folio.
> *
> * When this function successfully creates an inline extent, it returns 1 and
> - * unlocks all pages including locked_folio and starts I/O on them.
> - * (In reality inline extents are limited to a single page, so locked_folio is
> - * the only page handled anyway).
> + * unlocks all folios including locked_folio and starts I/O on them.
> + * (In reality inline extents are limited to a single block, so locked_folio is
> + * the only folio handled anyway).
> *
> - * When this function succeed and creates a normal extent, the page locking
> + * When this function succeed and creates a normal extent, the folio locking
> * status depends on the passed in flags:
> *
> - * - If @keep_locked is set, all pages are kept locked.
> - * - Else all pages except for @locked_folio are unlocked.
> + * - If COW_FILE_RANGE_KEEP_LOCKED flag is set, all folios are kept locked.
> + * - Else all folios except for @locked_folio are unlocked.
> *
> * When a failure happens in the second or later iteration of the
> * while-loop, the ordered extents created in previous iterations are cleaned up.
> @@ -1262,7 +1265,7 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
> static noinline int cow_file_range(struct btrfs_inode *inode,
> struct folio *locked_folio, u64 start,
> u64 end, u64 *done_offset,
> - bool keep_locked, bool no_inline)
> + unsigned long flags)
> {
> struct btrfs_root *root = inode->root;
> struct btrfs_fs_info *fs_info = root->fs_info;
> @@ -1290,7 +1293,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>
> inode_should_defrag(inode, start, end, num_bytes, SZ_64K);
>
> - if (!no_inline) {
> + if (!(flags & COW_FILE_RANGE_NO_INLINE)) {
I get that you are keeping the existing semantics, but if you are
bothering to refactor it, I think it would be nice to also get rid of
the double negative here. COW_FILE_RANGE_ALLOW_INLINE or TRY_INLINE
maybe?
> /* lets try to make an inline extent */
> ret = cow_file_range_inline(inode, locked_folio, start, end, 0,
> BTRFS_COMPRESS_NONE, NULL, false);
> @@ -1318,7 +1321,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
> * Do set the Ordered (Private2) bit so we know this page was properly
> * setup for writepage.
> */
> - page_ops = (keep_locked ? 0 : PAGE_UNLOCK);
> + page_ops = ((flags & COW_FILE_RANGE_KEEP_LOCKED) ? 0 : PAGE_UNLOCK);
> page_ops |= PAGE_SET_ORDERED;
>
> /*
> @@ -1685,7 +1688,7 @@ static noinline int run_delalloc_cow(struct btrfs_inode *inode,
>
> while (start <= end) {
> ret = cow_file_range(inode, locked_folio, start, end,
> - &done_offset, true, false);
> + &done_offset, COW_FILE_RANGE_KEEP_LOCKED);
> if (ret)
> return ret;
> extent_write_locked_range(&inode->vfs_inode, locked_folio,
> @@ -1767,8 +1770,8 @@ static int fallback_to_cow(struct btrfs_inode *inode,
> * is written out and unlocked directly and a normal NOCOW extent
> * doesn't work.
> */
> - ret = cow_file_range(inode, locked_folio, start, end, NULL, false,
> - true);
> + ret = cow_file_range(inode, locked_folio, start, end, NULL,
> + COW_FILE_RANGE_NO_INLINE);
> ASSERT(ret != 1);
> return ret;
> }
> @@ -2347,8 +2350,7 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct folio *locked_fol
> ret = run_delalloc_cow(inode, locked_folio, start, end, wbc,
> true);
> else
> - ret = cow_file_range(inode, locked_folio, start, end, NULL,
> - false, false);
> + ret = cow_file_range(inode, locked_folio, start, end, NULL, 0);
> return ret;
> }
>
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios
2025-07-20 6:29 ` [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios Qu Wenruo
@ 2025-07-21 20:32 ` Boris Burkov
0 siblings, 0 replies; 8+ messages in thread
From: Boris Burkov @ 2025-07-21 20:32 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Sun, Jul 20, 2025 at 03:59:10PM +0930, Qu Wenruo wrote:
> When hitting a large folio, btrfs_cleanup_ordered_extents() will get the
> same large folio multiple times, and clearing the same range again and
> again.
>
> Thankfully this is not causing anything wrong, just inefficiency.
>
> This is caused by the fact that we're iterating folios using the old
> page index, thus can hit the same large folio again and again.
>
> Enhance it by increasing @index to the index of the folio end, and only
> increase @index by 1 if we failed to grab a folio.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
> fs/btrfs/inode.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index fc47e234b729..5259bb8ec430 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -404,10 +404,12 @@ static inline void btrfs_cleanup_ordered_extents(struct btrfs_inode *inode,
>
> while (index <= end_index) {
> folio = filemap_get_folio(inode->vfs_inode.i_mapping, index);
> - index++;
> - if (IS_ERR(folio))
> + if (IS_ERR(folio)) {
> + index++;
> continue;
> + }
>
> + index = folio_end(folio) >> PAGE_SHIFT;
> /*
> * Here we just clear all Ordered bits for every page in the
> * range, then btrfs_mark_ordered_io_finished() will handle
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents()
2025-07-20 6:29 ` [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents() Qu Wenruo
@ 2025-07-21 20:36 ` Boris Burkov
0 siblings, 0 replies; 8+ messages in thread
From: Boris Burkov @ 2025-07-21 20:36 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Sun, Jul 20, 2025 at 03:59:11PM +0930, Qu Wenruo wrote:
> Inside nocow_one_range(), if the checksum cloning for data reloc inode
> failed, we call btrfs_cleanup_ordered_extents() to cleanup the just
> allocated ordered extents.
>
> But unlike extent_clear_unlock_delalloc(),
> btrfs_cleanup_ordered_extents() requires a length, not an inclusive end
> bytenr.
>
> This can be problematic, as the @end is normally way larger than @len.
>
> This means btrfs_cleanup_ordered_extents() can be called on folios
> out of the correct range, and if the out-of-range folio is under
> writeback, we can incorrectly clear the ordered flag of the folio, and
> trigger the DEBUG_WARN() inside btrfs_writepage_cow_fixup().
>
> Fix the wrong parameter with correct length instead.
>
> Fixes: 94f6c5c17e52 ("btrfs: move ordered extent cleanup to where they are allocated")
Oops, missed that last time :)
Thanks for the fix,
Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5259bb8ec430..6d9a8d8bea4c 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2018,7 +2018,7 @@ static int nocow_one_range(struct btrfs_inode *inode, struct folio *locked_folio
> * cleaered by the caller.
> */
> if (ret < 0)
> - btrfs_cleanup_ordered_extents(inode, file_pos, end);
> + btrfs_cleanup_ordered_extents(inode, file_pos, len);
> return ret;
> }
>
> --
> 2.50.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range()
2025-07-21 20:29 ` Boris Burkov
@ 2025-07-21 22:59 ` Qu Wenruo
0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2025-07-21 22:59 UTC (permalink / raw)
To: Boris Burkov; +Cc: linux-btrfs
在 2025/7/22 05:59, Boris Burkov 写道:
> On Sun, Jul 20, 2025 at 03:59:09PM +0930, Qu Wenruo wrote:
>> The function cow_file_range() has two boolean parameters, which is never
>> a good thing for eyes.
>>
>> Replace it with a single @flags parameter, with two flags:
>>
>> - COW_FILE_RANGE_NO_INLINE
>> - COW_FILE_RANGE_KEEP_LOCKED
>>
>> And since we're here, also update the comments of cow_file_range() to
>> replace the old "page" usage with "folio".
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
> Had a nit, but looks good overall. I think the flag approach is nice.
> Reviewed-by: Boris Burkov <boris@bur.io>
>
>> ---
>> fs/btrfs/inode.c | 32 +++++++++++++++++---------------
>> 1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index b77dd22b8cdb..fc47e234b729 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -72,6 +72,9 @@
>> #include "raid-stripe-tree.h"
>> #include "fiemap.h"
>>
>> +#define COW_FILE_RANGE_KEEP_LOCKED (1UL << 0)
>> +#define COW_FILE_RANGE_NO_INLINE (1UL << 1)
>> +
>> struct btrfs_iget_args {
>> u64 ino;
>> struct btrfs_root *root;
>> @@ -1243,18 +1246,18 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
>> * locked_folio is the folio that writepage had locked already. We use
>> * it to make sure we don't do extra locks or unlocks.
>> *
>> - * When this function fails, it unlocks all pages except @locked_folio.
>> + * When this function fails, it unlocks all folios except @locked_folio.
>> *
>> * When this function successfully creates an inline extent, it returns 1 and
>> - * unlocks all pages including locked_folio and starts I/O on them.
>> - * (In reality inline extents are limited to a single page, so locked_folio is
>> - * the only page handled anyway).
>> + * unlocks all folios including locked_folio and starts I/O on them.
>> + * (In reality inline extents are limited to a single block, so locked_folio is
>> + * the only folio handled anyway).
>> *
>> - * When this function succeed and creates a normal extent, the page locking
>> + * When this function succeed and creates a normal extent, the folio locking
>> * status depends on the passed in flags:
>> *
>> - * - If @keep_locked is set, all pages are kept locked.
>> - * - Else all pages except for @locked_folio are unlocked.
>> + * - If COW_FILE_RANGE_KEEP_LOCKED flag is set, all folios are kept locked.
>> + * - Else all folios except for @locked_folio are unlocked.
>> *
>> * When a failure happens in the second or later iteration of the
>> * while-loop, the ordered extents created in previous iterations are cleaned up.
>> @@ -1262,7 +1265,7 @@ u64 btrfs_get_extent_allocation_hint(struct btrfs_inode *inode, u64 start,
>> static noinline int cow_file_range(struct btrfs_inode *inode,
>> struct folio *locked_folio, u64 start,
>> u64 end, u64 *done_offset,
>> - bool keep_locked, bool no_inline)
>> + unsigned long flags)
>> {
>> struct btrfs_root *root = inode->root;
>> struct btrfs_fs_info *fs_info = root->fs_info;
>> @@ -1290,7 +1293,7 @@ static noinline int cow_file_range(struct btrfs_inode *inode,
>>
>> inode_should_defrag(inode, start, end, num_bytes, SZ_64K);
>>
>> - if (!no_inline) {
>> + if (!(flags & COW_FILE_RANGE_NO_INLINE)) {
>
> I get that you are keeping the existing semantics, but if you are
> bothering to refactor it, I think it would be nice to also get rid of
> the double negative here. COW_FILE_RANGE_ALLOW_INLINE or TRY_INLINE
> maybe?
I have considered this, but for the current 3 call sites, one doesn't
want inline, two want inline, thus INLINE is the more common pattern.
Thus I kept the existing scheme, to keep the minority call sites to use
extra flags.
Thanks,
Qu
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-21 22:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-20 6:29 [PATCH 0/3] btrfs: simple enhancement and fix for delalloc range Qu Wenruo
2025-07-20 6:29 ` [PATCH 1/3] btrfs: replace double boolean parameters of cow_file_range() Qu Wenruo
2025-07-21 20:29 ` Boris Burkov
2025-07-21 22:59 ` Qu Wenruo
2025-07-20 6:29 ` [PATCH 2/3] btrfs: make btrfs_cleanup_ordered_extents() to support large folios Qu Wenruo
2025-07-21 20:32 ` Boris Burkov
2025-07-20 6:29 ` [PATCH 3/3] btrfs: fix the wrong parameter for btrfs_cleanup_ordered_extents() Qu Wenruo
2025-07-21 20:36 ` Boris Burkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.