Re: [PATCH 0/3] Three folio-related btrfs fixes

[David Sterba <dsterba@suse.cz>]

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.