Re: [PATCH 09/11] generic/778: fix severe performance problems

["Darrick J. Wong" <djwong@kernel.org>]

On Sat, Nov 01, 2025 at 05:34:18PM +0800, Zorro Lang wrote:
> On Fri, Oct 31, 2025 at 10:47:34AM -0700, Darrick J. Wong wrote:
> > On Tue, Oct 21, 2025 at 09:27:31PM -0700, Darrick J. Wong wrote:
> > > On Tue, Oct 21, 2025 at 09:20:55PM -0700, Christoph Hellwig wrote:
> > > > On Tue, Oct 21, 2025 at 11:41:33AM -0700, Darrick J. Wong wrote:
> > > > > As a result, one loop through the test takes almost 4 minutes.  The test
> > > > > loops 20 times, so it runs for 80 minutes(!!) which is a really long
> > > > > time.
> > > > 
> > > > Heh.  I'm glade none of my usual test setups even supports atomics I
> > > > guess :)
> > > 
> > > FWIW the failure was on a regular xfs, no hw atomics.  So in theory
> > > you're affected, but only if you pulled the 20 Oct next branch.
> > > 
> > > > > So the first thing we do is observe that the giant slow loop is being
> > > > > run as a single thread on an empty filesystem.  Most of the time the
> > > > > allocator generates a mostly physically contiguous file.  We could
> > > > > fallocate the whole file instead of fallocating one block every other
> > > > > time through the loop.  This halves the setup time.
> > > > > 
> > > > > Next, we can also stuff the remaining pwrite commands into a bash array
> > > > > and only invoke xfs_io once every 128x through the loop.  This amortizes
> > > > > the xfs_io startup time, which reduces the test loop runtime to about 20
> > > > > seconds.
> > > > 
> > > > Wouldn't it make sense to adopt src/punch-alternating.c to also be
> > > > able to create unwritten extents instead of holes for the punched
> > > > range and run all of this from a C program?
> > > 
> > > For the write sizes it comes up with I'm guessing that this test will
> > > almost always be poking the software fallbacks so it probably doesn't
> > > matter if the file is full of holes.
> > 
> > ...and now running this with 32k-fsblocks reveals that the
> > atomic_write_loop code actually writes the wrong value into $tmp.aw and
> > only runs the loop once, so the test fails because dry_run thinks the
> > file size should be 0.
> > 
> > Also the cmds+=() line needs to insert its own -c or else you end up
> > writing huge files to $here.  Ooops.
> > 
> > Will send a v2 once the brownpaperbag testing finishes.
> 
> Hi Darrick,
> 
> JFYI:) If you don't mind, as I haven't seen your v2, I'll try to merge this patchset
> without this [09/11] at first. Then I can merge another patchset "[PATCH 0/3] generic/772:
> split and fix" which bases on this patchset, and give them a test.

Sounds good!

The 778 changes are going to take a while longer because it keeps
breaking down in weird ways on 64k fsblock filesystems.

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > > Otherwise this looks good:
> > > > 
> > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Thanks!
> > > 
> > > --D
> > > 
> > 
>