Re: [RFC PATCH 0/3]: Extreme fragmentation ahoy!

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Fri, Feb 08, 2019 at 07:34:33AM -0500, Brian Foster wrote:
> On Fri, Feb 08, 2019 at 01:47:30PM +1100, Dave Chinner wrote:
> > On Thu, Feb 07, 2019 at 10:52:43AM -0500, Brian Foster wrote:
> > > On Thu, Feb 07, 2019 at 04:39:41PM +1100, Dave Chinner wrote:
> > > > On Wed, Feb 06, 2019 at 09:21:14PM -0800, Darrick J. Wong wrote:
> > > > > On Thu, Feb 07, 2019 at 04:08:10PM +1100, Dave Chinner wrote:
> > > > > > Hi folks,
> > > > > > 
> > > > > > I've just finished analysing an IO trace from a application
> > > > > > generating an extreme filesystem fragmentation problem that started
> > > > > > with extent size hints and ended with spurious ENOSPC reports due to
> > > > > > massively fragmented files and free space. While the ENOSPC issue
> > > > > > looks to have previously been solved, I still wanted to understand
> > > > > > how the application had so comprehensively defeated extent size
> > > > > > hints as a method of avoiding file fragmentation.
> > ....
> > > > FWIW, I think the scope of the problem is quite widespread -
> > > > anything that does open/something/close repeatedly on a file that is
> > > > being written to with O_DSYNC or O_DIRECT appending writes will kill
> > > > the post-eof extent size hint allocated space. That's why I suspect
> > > > we need to think about not trimming by default and trying to
> > > > enumerating only the cases that need to trim eof blocks.
> > > > 
> > > 
> > > To further this point.. I think the eofblocks scanning stuff came long
> > > after the speculative preallocation code and associated release time
> > > post-eof truncate.
> > 
> > Yes, I cribed a bit of the history of the xfs_release() behaviour
> > on #xfs yesterday afternoon:
> > 
> >  <djwong>	dchinner: feel free to ignore this until tomorrow if you want, but /me wonders why we'd want to free the eofblocks at close time at all, instead of waiting for inactivation/enospc/background reaper to do it?
> >  <dchinner>	historic. People doing operations then complaining du didn't match ls
> >  <dchinner>	stuff like that
> >  <dchinner>	There used to be a open file cache in XFS - we'd know exactly when the last reference went away and trim it then
> >  <dchinner>	but that went away when NFS and the dcache got smarter about file handle conversion
> >  <dchinner>	(i.e. that's how we used to make nfs not suck)
> >  <dchinner>	that's when we started doing work in ->release
> >  <dchinner>	it was close enough to "last close" for most workloads it made no difference.
> >  <dchinner>	Except for concurrent NFS writes into the same directory
> >  <dchinner>	and now there's another pathological application that triggers problems
> >  <dchinner>	The NFS exception was prior to having thebackground reaper
> >  <dchinner>	as these things goes the background reaper is relatively recent functionality
> >  <dchinner>	so perhaps we should just leave it to "inode cache expiry or background reaping" and not do it on close at al
> > 
> 
> Thanks.
> 
> > > I think the background scanning was initially an
> > > enhancement to deal with things like the dirty release optimization
> > > leaving these blocks around longer and being able to free up this
> > > accumulated space when we're at -ENOSPC conditions.
> > 
> > Yes, amongst other things like slow writes keeping the file open
> > forever.....
> > 
> > > Now that we have the
> > > scanning mechanism in place (and a 5 minute default background scan,
> > > which really isn't all that long), it might be reasonable to just drop
> > > the release time truncate completely and only trim post-eof blocks via
> > > the bg scan or reclaim paths.
> > 
> > Yeah, that's kinda the question I'm asking here. What's the likely
> > impact of not trimming EOF blocks at least on close apart from
> > people complaining about df/ls not matching du?
> > 
> 
> Ok. ISTM it's just a continuation of the same "might confuse some users"
> scenario that pops up occasionally. It also seems that kind of thing has
> died down as either most people don't really know or care about the
> transient state or are just more familiar with it at this point. IME,
> complex applications that depend on block ownership stats (userspace
> filesystems for example) already have to account for speculative
> preallocation with XFS, so tweaking the semantics of the optimization
> shouldn't really have much of an impact that I can tell so long as the
> broader/long-term behavior doesn't change[1].
> 
> I suppose there are all kinds of other applications that are technically
> affected by dropping the release time trim (simple file copies, archive
> extraction, etc.), but it's not clear to me that matters so long as we
> have effective bg and -ENOSPC scans. The only thing I can think of so
> far is whether we should consider changes to the bg scan heuristics to
> accommodate scenarios currently covered by the release time trim. For
> example, the release time scan doesn't consider whether the file is
> dirty or not while the bg scan always skips "active" files.

I wrote a quick and dirty fstest that writes 999 files between 128k and
256k in size, to simulate untarring onto a filesystem.  No fancy
preallocation, just buffered writes.  I patched my kernel to skip the
posteof block freeing in xfs_release, so the preallocations get freed by
inode inactivation.  Then the freespace histogram looks like:

+   from      to extents  blocks    pct
+      1       1      36      36   0.00
+      2       3      69     175   0.01
+      4       7     122     698   0.02
+      8      15     237    2691   0.08
+     16      31       1      16   0.00
+     32      63     500   27843   0.88
+ 524288  806272       4 3141225  99.01

Pretty gnarly. :)  By comparison, a stock upstream kernel:

+   from      to extents  blocks    pct
+ 524288  806272       4 3172579 100.00

That's 969 free extents vs. 4, on a fs with 999 new files... which is
pretty bad.  Dave also suggessted on IRC that maybe this should be a
little smarter -- possibly skipping the posteof removal only if the
filesystem has sunit/swidth set, or if the inode has extent size hints,
or whatever. :)

--D

> Brian
> 
> [1] Which isn't the case for a change to not bg trim files with extent
> size hints.
> 
> > I don't really care about that anymore because, well, reflink/dedupe
> > completely break any remaining assumption that du reported space
> > consumption is related to the file size (if sparse files wasn't
> > enough of a hint arlready)....
> > 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com