* [PATCH 0/3] Three folio-related btrfs fixes
@ 2026-05-22 18:14 Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 1/3] Revert "btrfs: fix the file offset calculation inside btrfs_decompress_buf2page()" Matthew Wilcox (Oracle)
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Matthew Wilcox (Oracle) @ 2026-05-22 18:14 UTC (permalink / raw)
To: Chris Mason
Cc: Matthew Wilcox (Oracle), David Sterba, linux-btrfs, Qu Wenruo,
Boris Burkov, Filipe Manana, Sweet Tea Dorminy
All of these were discovered through auditing the (mis)uses of the folio
API throughout the kernel; I haven't been specifically reviewing btrfs,
nor testing.
Matthew Wilcox (Oracle) (3):
Revert "btrfs: fix the file offset calculation inside
btrfs_decompress_buf2page()"
btrfs: Use folio_put()
btrfs: Remove use of bv_page
fs/btrfs/compression.c | 18 +-----------------
fs/btrfs/extent_io.c | 6 +++---
fs/btrfs/inode.c | 2 +-
3 files changed, 5 insertions(+), 21 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] Revert "btrfs: fix the file offset calculation inside btrfs_decompress_buf2page()"
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
@ 2026-05-22 18:14 ` Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 2/3] btrfs: Use folio_put() Matthew Wilcox (Oracle)
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Matthew Wilcox (Oracle) @ 2026-05-22 18:14 UTC (permalink / raw)
To: Chris Mason
Cc: Matthew Wilcox (Oracle), David Sterba, linux-btrfs, Qu Wenruo,
Boris Burkov, Filipe Manana, Sweet Tea Dorminy
It seems that af566bdaff54 was tested against a tree which did not
contain commit 12851bd921d4 ("fs: Turn page_offset() into a wrapper
around folio_pos()). Unfortunately it has a bug of its own; on 32-bit
systems, shifting by PAGE_SHIFT will overflow on files larger than 4GiB.
Since page_offset() is now fixed, just revert af566bdaff54.
Fixes: af566bdaff54 (btrfs: fix the file offset calculation inside btrfs_decompress_buf2page())
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/btrfs/compression.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index a02b62e0a8f3..2ceb5661e071 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1191,22 +1191,6 @@ void __cold btrfs_exit_compress(void)
bioset_exit(&btrfs_compressed_bioset);
}
-/*
- * The bvec is a single page bvec from a bio that contains folios from a filemap.
- *
- * Since the folio may be a large one, and if the bv_page is not a head page of
- * a large folio, then page->index is unreliable.
- *
- * Thus we need this helper to grab the proper file offset.
- */
-static u64 file_offset_from_bvec(const struct bio_vec *bvec)
-{
- const struct page *page = bvec->bv_page;
- const struct folio *folio = page_folio(page);
-
- return (page_pgoff(folio, page) << PAGE_SHIFT) + bvec->bv_offset;
-}
-
/*
* Copy decompressed data from working buffer to pages.
*
@@ -1259,7 +1243,7 @@ int btrfs_decompress_buf2page(const char *buf, u32 buf_len,
* cb->start may underflow, but subtracting that value can still
* give us correct offset inside the full decompressed extent.
*/
- bvec_offset = file_offset_from_bvec(&bvec) - cb->start;
+ bvec_offset = page_offset(bvec.bv_page) + bvec.bv_offset - cb->start;
/* Haven't reached the bvec range, exit */
if (decompressed + buf_len <= bvec_offset)
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] btrfs: Use folio_put()
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 1/3] Revert "btrfs: fix the file offset calculation inside btrfs_decompress_buf2page()" Matthew Wilcox (Oracle)
@ 2026-05-22 18:14 ` Matthew Wilcox (Oracle)
2026-05-22 21:15 ` Boris Burkov
2026-05-22 18:14 ` [PATCH 3/3] btrfs: Remove use of bv_page Matthew Wilcox (Oracle)
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Matthew Wilcox (Oracle) @ 2026-05-22 18:14 UTC (permalink / raw)
To: Chris Mason
Cc: Matthew Wilcox (Oracle), David Sterba, linux-btrfs, Qu Wenruo,
Boris Burkov, Filipe Manana, Sweet Tea Dorminy
Calling __free_page() on folio_page() happens to work today, but
won't always. Besides, it's far simpler to call folio_put().
Fixes: 082d5bb9b336 (btrfs: migrate extent_buffer::pages[] to folio)
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
fs/btrfs/extent_io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 2275189b7860..478b7d936aa4 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3394,8 +3394,8 @@ static int attach_eb_folio_to_filemap(struct extent_buffer *eb, int i,
finish:
spin_lock(&mapping->i_private_lock);
if (existing_folio && btrfs_meta_is_subpage(fs_info)) {
- /* We're going to reuse the existing page, can drop our folio now. */
- __free_page(folio_page(eb->folios[i], 0));
+ /* We're going to reuse the existing folio, can drop our folio now. */
+ folio_put(eb->folios[i]);
eb->folios[i] = existing_folio;
} else if (existing_folio) {
struct extent_buffer *existing_eb;
@@ -3410,7 +3410,7 @@ static int attach_eb_folio_to_filemap(struct extent_buffer *eb, int i,
return 1;
}
/* The extent buffer no longer exists, we can reuse the folio. */
- __free_page(folio_page(eb->folios[i], 0));
+ folio_put(eb->folios[i]);
eb->folios[i] = existing_folio;
}
eb->folio_size = folio_size(eb->folios[i]);
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] btrfs: Remove use of bv_page
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 1/3] Revert "btrfs: fix the file offset calculation inside btrfs_decompress_buf2page()" Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 2/3] btrfs: Use folio_put() Matthew Wilcox (Oracle)
@ 2026-05-22 18:14 ` Matthew Wilcox (Oracle)
2026-05-22 21:20 ` [PATCH 0/3] Three folio-related btrfs fixes Boris Burkov
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Matthew Wilcox (Oracle) @ 2026-05-22 18:14 UTC (permalink / raw)
To: Chris Mason
Cc: Matthew Wilcox (Oracle), David Sterba, linux-btrfs, Qu Wenruo,
Boris Burkov, Filipe Manana, Sweet Tea Dorminy
This is open-coded bvec_phys()
Fixes: 6f706f34fc4c (btrfs: switch to btrfs_compress_bio() interface for compressed writes)
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
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 75136a172710..10ca76b177b3 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -842,7 +842,7 @@ static struct folio *compressed_bio_last_folio(struct compressed_bio *cb)
ASSERT(bio->bi_vcnt);
bvec = &bio->bi_io_vec[bio->bi_vcnt - 1];
- paddr = page_to_phys(bvec->bv_page) + bvec->bv_offset + bvec->bv_len - 1;
+ paddr = bvec_phys(bvec) + bvec->bv_len - 1;
return page_folio(phys_to_page(paddr));
}
--
2.47.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] btrfs: Use folio_put()
2026-05-22 18:14 ` [PATCH 2/3] btrfs: Use folio_put() Matthew Wilcox (Oracle)
@ 2026-05-22 21:15 ` Boris Burkov
0 siblings, 0 replies; 16+ messages in thread
From: Boris Burkov @ 2026-05-22 21:15 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Filipe Manana,
Sweet Tea Dorminy
On Fri, May 22, 2026 at 07:14:08PM +0100, Matthew Wilcox (Oracle) wrote:
> Calling __free_page() on folio_page() happens to work today, but
> won't always. Besides, it's far simpler to call folio_put().
>
> Fixes: 082d5bb9b336 (btrfs: migrate extent_buffer::pages[] to folio)
I'm used to Fixes: being for bugs rather than cleanups. I obviously don't
mind, but it just stood out to me.
Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/btrfs/extent_io.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 2275189b7860..478b7d936aa4 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -3394,8 +3394,8 @@ static int attach_eb_folio_to_filemap(struct extent_buffer *eb, int i,
> finish:
> spin_lock(&mapping->i_private_lock);
> if (existing_folio && btrfs_meta_is_subpage(fs_info)) {
> - /* We're going to reuse the existing page, can drop our folio now. */
> - __free_page(folio_page(eb->folios[i], 0));
> + /* We're going to reuse the existing folio, can drop our folio now. */
> + folio_put(eb->folios[i]);
> eb->folios[i] = existing_folio;
> } else if (existing_folio) {
> struct extent_buffer *existing_eb;
> @@ -3410,7 +3410,7 @@ static int attach_eb_folio_to_filemap(struct extent_buffer *eb, int i,
> return 1;
> }
> /* The extent buffer no longer exists, we can reuse the folio. */
> - __free_page(folio_page(eb->folios[i], 0));
> + folio_put(eb->folios[i]);
> eb->folios[i] = existing_folio;
> }
> eb->folio_size = folio_size(eb->folios[i]);
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
` (2 preceding siblings ...)
2026-05-22 18:14 ` [PATCH 3/3] btrfs: Remove use of bv_page Matthew Wilcox (Oracle)
@ 2026-05-22 21:20 ` Boris Burkov
2026-05-22 21:57 ` Qu Wenruo
2026-05-22 21:47 ` Qu Wenruo
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Boris Burkov @ 2026-05-22 21:20 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Filipe Manana,
Sweet Tea Dorminy
On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> All of these were discovered through auditing the (mis)uses of the folio
> API throughout the kernel; I haven't been specifically reviewing btrfs,
> nor testing.
I ran this through my btrfs fstests setup and observed no regressions.
However, that is on x86 systems with 4k blocksize so it was not exercising
subpage block size directly. Which means the revert is still relatively
untested. I'll try to get a proper subpage test done if Qu doesn't beat
me to it.
With that said,
Reviewed-by: Boris Burkov <boris@bur.io>
Tested-by: Boris Burkov <boris@bur.io>
>
> Matthew Wilcox (Oracle) (3):
> Revert "btrfs: fix the file offset calculation inside
> btrfs_decompress_buf2page()"
> btrfs: Use folio_put()
> btrfs: Remove use of bv_page
>
> fs/btrfs/compression.c | 18 +-----------------
> fs/btrfs/extent_io.c | 6 +++---
> fs/btrfs/inode.c | 2 +-
> 3 files changed, 5 insertions(+), 21 deletions(-)
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
` (3 preceding siblings ...)
2026-05-22 21:20 ` [PATCH 0/3] Three folio-related btrfs fixes Boris Burkov
@ 2026-05-22 21:47 ` Qu Wenruo
2026-05-22 22:38 ` David Sterba
2026-05-25 12:02 ` David Sterba
6 siblings, 0 replies; 16+ messages in thread
From: Qu Wenruo @ 2026-05-22 21:47 UTC (permalink / raw)
To: Matthew Wilcox (Oracle), Chris Mason
Cc: David Sterba, linux-btrfs, Boris Burkov, Filipe Manana,
Sweet Tea Dorminy
在 2026/5/23 03:44, Matthew Wilcox (Oracle) 写道:
> All of these were discovered through auditing the (mis)uses of the folio
> API throughout the kernel; I haven't been specifically reviewing btrfs,
> nor testing.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
>
> Matthew Wilcox (Oracle) (3):
> Revert "btrfs: fix the file offset calculation inside
> btrfs_decompress_buf2page()"
> btrfs: Use folio_put()
> btrfs: Remove use of bv_page
>
> fs/btrfs/compression.c | 18 +-----------------
> fs/btrfs/extent_io.c | 6 +++---
> fs/btrfs/inode.c | 2 +-
> 3 files changed, 5 insertions(+), 21 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 21:20 ` [PATCH 0/3] Three folio-related btrfs fixes Boris Burkov
@ 2026-05-22 21:57 ` Qu Wenruo
2026-05-22 22:07 ` Boris Burkov
0 siblings, 1 reply; 16+ messages in thread
From: Qu Wenruo @ 2026-05-22 21:57 UTC (permalink / raw)
To: Boris Burkov, Matthew Wilcox (Oracle)
Cc: Chris Mason, David Sterba, linux-btrfs, Filipe Manana,
Sweet Tea Dorminy
在 2026/5/23 06:50, Boris Burkov 写道:
> On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
>> All of these were discovered through auditing the (mis)uses of the folio
>> API throughout the kernel; I haven't been specifically reviewing btrfs,
>> nor testing.
>
> I ran this through my btrfs fstests setup and observed no regressions.
> However, that is on x86 systems with 4k blocksize so it was not exercising
> subpage block size directly.
BTW, on x86_64 it's very common to go large folios thus going through
subpage routine.
E.g. just doing something like "pwrite -b 64k 0 64k" will create a 64K
large folio thus exercising subpage routing.
Although a 64K page size run would definitely help.
Thanks,
Qu
> Which means the revert is still relatively
> untested. I'll try to get a proper subpage test done if Qu doesn't beat
> me to it.
>
> With that said,
> Reviewed-by: Boris Burkov <boris@bur.io>
> Tested-by: Boris Burkov <boris@bur.io>
>
>>
>> Matthew Wilcox (Oracle) (3):
>> Revert "btrfs: fix the file offset calculation inside
>> btrfs_decompress_buf2page()"
>> btrfs: Use folio_put()
>> btrfs: Remove use of bv_page
>>
>> fs/btrfs/compression.c | 18 +-----------------
>> fs/btrfs/extent_io.c | 6 +++---
>> fs/btrfs/inode.c | 2 +-
>> 3 files changed, 5 insertions(+), 21 deletions(-)
>>
>> --
>> 2.47.3
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 21:57 ` Qu Wenruo
@ 2026-05-22 22:07 ` Boris Burkov
0 siblings, 0 replies; 16+ messages in thread
From: Boris Burkov @ 2026-05-22 22:07 UTC (permalink / raw)
To: Qu Wenruo
Cc: Matthew Wilcox (Oracle), Chris Mason, David Sterba, linux-btrfs,
Filipe Manana, Sweet Tea Dorminy
On Sat, May 23, 2026 at 07:27:53AM +0930, Qu Wenruo wrote:
>
>
> 在 2026/5/23 06:50, Boris Burkov 写道:
> > On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> > > All of these were discovered through auditing the (mis)uses of the folio
> > > API throughout the kernel; I haven't been specifically reviewing btrfs,
> > > nor testing.
> >
> > I ran this through my btrfs fstests setup and observed no regressions.
> > However, that is on x86 systems with 4k blocksize so it was not exercising
> > subpage block size directly.
>
> BTW, on x86_64 it's very common to go large folios thus going through
> subpage routine.
>
> E.g. just doing something like "pwrite -b 64k 0 64k" will create a 64K large
> folio thus exercising subpage routing.
>
> Although a 64K page size run would definitely help.
I must have missed this detail in your patch set that dropped
experimental for large folios. By default on for-next with no additional
config we get large folios now? Sweet! I'll do some tracing to get used
to it :)
>
> Thanks,
> Qu
>
> > Which means the revert is still relatively
> > untested. I'll try to get a proper subpage test done if Qu doesn't beat
> > me to it.
> >
> > With that said,
> > Reviewed-by: Boris Burkov <boris@bur.io>
> > Tested-by: Boris Burkov <boris@bur.io>
> >
> > >
> > > Matthew Wilcox (Oracle) (3):
> > > Revert "btrfs: fix the file offset calculation inside
> > > btrfs_decompress_buf2page()"
> > > btrfs: Use folio_put()
> > > btrfs: Remove use of bv_page
> > >
> > > fs/btrfs/compression.c | 18 +-----------------
> > > fs/btrfs/extent_io.c | 6 +++---
> > > fs/btrfs/inode.c | 2 +-
> > > 3 files changed, 5 insertions(+), 21 deletions(-)
> > >
> > > --
> > > 2.47.3
> > >
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
` (4 preceding siblings ...)
2026-05-22 21:47 ` Qu Wenruo
@ 2026-05-22 22:38 ` David Sterba
2026-05-22 23:56 ` Matthew Wilcox
2026-05-25 12:02 ` David Sterba
6 siblings, 1 reply; 16+ messages in thread
From: David Sterba @ 2026-05-22 22:38 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> All of these were discovered through auditing the (mis)uses of the folio
> API throughout the kernel; I haven't been specifically reviewing btrfs,
> nor testing.
>
> Matthew Wilcox (Oracle) (3):
> Revert "btrfs: fix the file offset calculation inside
> btrfs_decompress_buf2page()"
Why do you do the revert? It does not seem suitable for this case, we
use reverts if we want to get back to the original behaviour, or when
removing a patch from a long series to avoid a rebase. Merging the first
and second patch makes more sense to me.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 22:38 ` David Sterba
@ 2026-05-22 23:56 ` Matthew Wilcox
2026-05-23 14:06 ` David Sterba
0 siblings, 1 reply; 16+ messages in thread
From: Matthew Wilcox @ 2026-05-22 23:56 UTC (permalink / raw)
To: David Sterba
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Sat, May 23, 2026 at 12:38:55AM +0200, David Sterba wrote:
> On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> > All of these were discovered through auditing the (mis)uses of the folio
> > API throughout the kernel; I haven't been specifically reviewing btrfs,
> > nor testing.
> >
> > Matthew Wilcox (Oracle) (3):
> > Revert "btrfs: fix the file offset calculation inside
> > btrfs_decompress_buf2page()"
>
> Why do you do the revert? It does not seem suitable for this case, we
> use reverts if we want to get back to the original behaviour, or when
> removing a patch from a long series to avoid a rebase. Merging the first
> and second patch makes more sense to me.
The revert is for a patch which was merged into v6.16. Since the fix
for page_offset() was merged into v6.15, af566bdaff54 should never have
been merged, and it can safely be reverted.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 23:56 ` Matthew Wilcox
@ 2026-05-23 14:06 ` David Sterba
2026-05-24 20:05 ` Matthew Wilcox
0 siblings, 1 reply; 16+ messages in thread
From: David Sterba @ 2026-05-23 14:06 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Sat, May 23, 2026 at 12:56:29AM +0100, Matthew Wilcox wrote:
> On Sat, May 23, 2026 at 12:38:55AM +0200, David Sterba wrote:
> > On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> > > All of these were discovered through auditing the (mis)uses of the folio
> > > API throughout the kernel; I haven't been specifically reviewing btrfs,
> > > nor testing.
> > >
> > > Matthew Wilcox (Oracle) (3):
> > > Revert "btrfs: fix the file offset calculation inside
> > > btrfs_decompress_buf2page()"
> >
> > Why do you do the revert? It does not seem suitable for this case, we
> > use reverts if we want to get back to the original behaviour, or when
> > removing a patch from a long series to avoid a rebase. Merging the first
> > and second patch makes more sense to me.
>
> The revert is for a patch which was merged into v6.16. Since the fix
> for page_offset() was merged into v6.15, af566bdaff54 should never have
> been merged, and it can safely be reverted.
That it can be safely/cleanly reverted does not matter, you could say
that about many other patches. If the code was wrong then fix it in a
"forward" manner, no need for revert, namely for code from a year ago.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-23 14:06 ` David Sterba
@ 2026-05-24 20:05 ` Matthew Wilcox
2026-05-24 23:16 ` Qu Wenruo
2026-05-25 10:28 ` David Sterba
0 siblings, 2 replies; 16+ messages in thread
From: Matthew Wilcox @ 2026-05-24 20:05 UTC (permalink / raw)
To: David Sterba
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Sat, May 23, 2026 at 04:06:01PM +0200, David Sterba wrote:
> On Sat, May 23, 2026 at 12:56:29AM +0100, Matthew Wilcox wrote:
> > On Sat, May 23, 2026 at 12:38:55AM +0200, David Sterba wrote:
> > > On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> > > > All of these were discovered through auditing the (mis)uses of the folio
> > > > API throughout the kernel; I haven't been specifically reviewing btrfs,
> > > > nor testing.
> > > >
> > > > Matthew Wilcox (Oracle) (3):
> > > > Revert "btrfs: fix the file offset calculation inside
> > > > btrfs_decompress_buf2page()"
> > >
> > > Why do you do the revert? It does not seem suitable for this case, we
> > > use reverts if we want to get back to the original behaviour, or when
> > > removing a patch from a long series to avoid a rebase. Merging the first
> > > and second patch makes more sense to me.
> >
> > The revert is for a patch which was merged into v6.16. Since the fix
> > for page_offset() was merged into v6.15, af566bdaff54 should never have
> > been merged, and it can safely be reverted.
>
> That it can be safely/cleanly reverted does not matter, you could say
> that about many other patches. If the code was wrong then fix it in a
> "forward" manner, no need for revert, namely for code from a year ago.
The patch should never have been merged. It didn't make sense at
the time. Reverting it is the right approach. There is no "forward"
to move it to, other than converting the code to use folios instead of
pages, which I'm not signing up for right now as that's more involved.
The fact that it's been in for a year with nobody reporting the bug just
shows the lack of 32-bit testing (and users).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-24 20:05 ` Matthew Wilcox
@ 2026-05-24 23:16 ` Qu Wenruo
2026-05-25 10:28 ` David Sterba
1 sibling, 0 replies; 16+ messages in thread
From: Qu Wenruo @ 2026-05-24 23:16 UTC (permalink / raw)
To: Matthew Wilcox, David Sterba
Cc: Chris Mason, David Sterba, linux-btrfs, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
在 2026/5/25 05:35, Matthew Wilcox 写道:
> On Sat, May 23, 2026 at 04:06:01PM +0200, David Sterba wrote:
>> On Sat, May 23, 2026 at 12:56:29AM +0100, Matthew Wilcox wrote:
>>> On Sat, May 23, 2026 at 12:38:55AM +0200, David Sterba wrote:
>>>> On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
>>>>> All of these were discovered through auditing the (mis)uses of the folio
>>>>> API throughout the kernel; I haven't been specifically reviewing btrfs,
>>>>> nor testing.
>>>>>
>>>>> Matthew Wilcox (Oracle) (3):
>>>>> Revert "btrfs: fix the file offset calculation inside
>>>>> btrfs_decompress_buf2page()"
>>>>
>>>> Why do you do the revert? It does not seem suitable for this case, we
>>>> use reverts if we want to get back to the original behaviour, or when
>>>> removing a patch from a long series to avoid a rebase. Merging the first
>>>> and second patch makes more sense to me.
>>>
>>> The revert is for a patch which was merged into v6.16. Since the fix
>>> for page_offset() was merged into v6.15, af566bdaff54 should never have
>>> been merged, and it can safely be reverted.
>>
>> That it can be safely/cleanly reverted does not matter, you could say
>> that about many other patches. If the code was wrong then fix it in a
>> "forward" manner, no need for revert, namely for code from a year ago.
>
> The patch should never have been merged. It didn't make sense at
> the time.
You're completely right, and it's all my fault.
The background is more complex, and making the patch more unnecessary.
Firstly that patch is sent and merged just before v6.15-rc1 came out, at
that time our development branch is based on the late v6.14-rc tags.
Thus the proper mm fix would not show up until v6.15-rc1 was out.
At that time I was testing btrfs large folios support, shaking out bugs
and sent out fixes as preparation patches.
Without that upstream fix showing in our development branch, the patch
made sense and got reviewed and queued, finally got upstreamed in the
next cycle (v6.16-rc1).
However just days before -rc1 came out, I got the reply from you
answering the question I submitted to the MM list
(https://lore.kernel.org/linux-btrfs/Z-7I9hOcGzQMV3hq@casper.infradead.org/),
which already shows the upstream commit.
At that time I should have removed the patch from our development
branch, but due to other reasons (I guess it was again the
update/testing of btrfs large folio support), I didn't notice that, and
lost the final chance to remove the patch from queue.
Finally what makes things worse is, btrfs large folio support came way
later than all those hassles.
v6.15 fixed the page_offset() bug for non-head page of a large folio.
v6.16 came with the btrfs' local fix, which is unnecessary.
v6.17 came with btrfs' experimental large folio support.
This means, there is no way to hit a large btrfs folio anyway before
v6.17, thus the local fix is not helping but only introducing an
overflow bug for 32bit systems.
> Reverting it is the right approach.
Yes, reverting is correct, it fixes a bug (overflow) that is introduced
by that patch, and there is no better way other than revert.
Sure you can do a minimal fix by just fixing the type before left
shifting, but that won't resolve the fact that page_offset() is already
fixed.
And the revert is also safe for backport.
The only affected branch is v6.18, and at that branch there is already
the upstream proper fix.
> There is no "forward"
> to move it to, other than converting the code to use folios instead of
> pages, which I'm not signing up for right now as that's more involved.
>
> The fact that it's been in for a year with nobody reporting the bug just
> shows the lack of 32-bit testing (and users).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-24 20:05 ` Matthew Wilcox
2026-05-24 23:16 ` Qu Wenruo
@ 2026-05-25 10:28 ` David Sterba
1 sibling, 0 replies; 16+ messages in thread
From: David Sterba @ 2026-05-25 10:28 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Sun, May 24, 2026 at 09:05:37PM +0100, Matthew Wilcox wrote:
> On Sat, May 23, 2026 at 04:06:01PM +0200, David Sterba wrote:
> > On Sat, May 23, 2026 at 12:56:29AM +0100, Matthew Wilcox wrote:
> > > On Sat, May 23, 2026 at 12:38:55AM +0200, David Sterba wrote:
> > > > On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> > > > > All of these were discovered through auditing the (mis)uses of the folio
> > > > > API throughout the kernel; I haven't been specifically reviewing btrfs,
> > > > > nor testing.
> > > > >
> > > > > Matthew Wilcox (Oracle) (3):
> > > > > Revert "btrfs: fix the file offset calculation inside
> > > > > btrfs_decompress_buf2page()"
> > > >
> > > > Why do you do the revert? It does not seem suitable for this case, we
> > > > use reverts if we want to get back to the original behaviour, or when
> > > > removing a patch from a long series to avoid a rebase. Merging the first
> > > > and second patch makes more sense to me.
> > >
> > > The revert is for a patch which was merged into v6.16. Since the fix
> > > for page_offset() was merged into v6.15, af566bdaff54 should never have
> > > been merged, and it can safely be reverted.
> >
> > That it can be safely/cleanly reverted does not matter, you could say
> > that about many other patches. If the code was wrong then fix it in a
> > "forward" manner, no need for revert, namely for code from a year ago.
>
> The patch should never have been merged. It didn't make sense at
> the time. Reverting it is the right approach. There is no "forward"
> to move it to, other than converting the code to use folios instead of
> pages, which I'm not signing up for right now as that's more involved.
I've read the patches again and you're right. The part I overlooked was
that 2nd and 3rd didn't change the same code, so it's reverting to the
previous state (without a followup).
> The fact that it's been in for a year with nobody reporting the bug just
> shows the lack of 32-bit testing (and users).
Yeah, we don't have 32bit setups for testing and distros have been
gradually removing their 32bit versions and I don't remember any recent
report. In the past it's been mostly from 3rd party testing, not users.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] Three folio-related btrfs fixes
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
` (5 preceding siblings ...)
2026-05-22 22:38 ` David Sterba
@ 2026-05-25 12:02 ` David Sterba
6 siblings, 0 replies; 16+ messages in thread
From: David Sterba @ 2026-05-25 12:02 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Chris Mason, David Sterba, linux-btrfs, Qu Wenruo, Boris Burkov,
Filipe Manana, Sweet Tea Dorminy
On Fri, May 22, 2026 at 07:14:06PM +0100, Matthew Wilcox (Oracle) wrote:
> All of these were discovered through auditing the (mis)uses of the folio
> API throughout the kernel; I haven't been specifically reviewing btrfs,
> nor testing.
>
> Matthew Wilcox (Oracle) (3):
> Revert "btrfs: fix the file offset calculation inside
> btrfs_decompress_buf2page()"
> btrfs: Use folio_put()
> btrfs: Remove use of bv_page
Added to for-next, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-05-25 12:02 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22 18:14 [PATCH 0/3] Three folio-related btrfs fixes Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 1/3] Revert "btrfs: fix the file offset calculation inside btrfs_decompress_buf2page()" Matthew Wilcox (Oracle)
2026-05-22 18:14 ` [PATCH 2/3] btrfs: Use folio_put() Matthew Wilcox (Oracle)
2026-05-22 21:15 ` Boris Burkov
2026-05-22 18:14 ` [PATCH 3/3] btrfs: Remove use of bv_page Matthew Wilcox (Oracle)
2026-05-22 21:20 ` [PATCH 0/3] Three folio-related btrfs fixes Boris Burkov
2026-05-22 21:57 ` Qu Wenruo
2026-05-22 22:07 ` Boris Burkov
2026-05-22 21:47 ` Qu Wenruo
2026-05-22 22:38 ` David Sterba
2026-05-22 23:56 ` Matthew Wilcox
2026-05-23 14:06 ` David Sterba
2026-05-24 20:05 ` Matthew Wilcox
2026-05-24 23:16 ` Qu Wenruo
2026-05-25 10:28 ` David Sterba
2026-05-25 12:02 ` David Sterba
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.