Re: [PATCH] btrfs: adjust subpage bit start based on sectorsize

[Josef Bacik <josef@toxicpanda.com>]

On Wed, Apr 16, 2025 at 09:19:37AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2025/4/16 01:46, Boris Burkov 写道:
> > On Tue, Apr 15, 2025 at 08:07:08AM +0930, Qu Wenruo wrote:
> [...]
> > > > 
> > > > The problem is, we can not ensure all extent buffers are nodesize aligned.
> > > > 
> > 
> > In theory because the allocator can do whatever it wants, or in practice
> > because of mixed block groups? It seems to me that in practice without
> > mixed block groups they ought to always be aligned.
> 
> Because we may have some old converted btrfs, whose allocater may not ensure
> nodesize aligned bytenr.
> 
> For subpage we can still support non-aligned tree blocks as long as they do
> not cross boundary.
> 
> I know this is over-complicated and prefer to reject them all, but such a
> change will need quite some time to reach end users.
> 
> > 
> > > > If we have an eb whose bytenr is only block aligned but not node size
> > > > aligned, this will lead to the same problem.
> > > > 
> > 
> > But then the existing code for the non error path is broken, right?
> > How was this intended to work? Is there any correct way to loop over the
> > ebs in a folio when nodesize < pagesize? Your proposed gang lookup?
> > 
> > I guess to put my question a different way, was it intentional to mix
> > the increment size in the two codepaths in this function?
> 
> Yes, that's intended, consider the following case:
> 
> 32K page size, 16K node size.
> 
> 0    4K     8K    16K    20K    24K     28K      32K
> |           |///////////////////|                |
> 
> In above case, for offset 0 and 4K, we didn't find any dirty block, and skip
> to next block.
> 
> For 8K, we found an eb, submit it, and jump to 24K, and that's the expected
> behavior.
> 
> But on the other hand, if at offset 0 we increase the offset by 16K, we will
> never be able to grab the eb at 8K.

Right, but this can't actually happen can it?  I mean it could happen the first
eb, but as soon as we allocate eb->start 24k we would fail check_eb_alignment()
because it straddles a folio and then the fs would flipe RO.  So I don't think
this is a problem in general.  Thanks,

Josef