Re: [PATCH 1/5] xfs: don't treat append-only files as having preallocations

[Dave Chinner <david@fromorbit.com>]

On Wed, Jun 12, 2024 at 10:46:48AM -0700, Darrick J. Wong wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> The XFS XFS_DIFLAG_APPEND maps to the VFS S_APPEND flag, which forbids
> writes that don't append at the current EOF.
> 
> But the commit originally adding XFS_DIFLAG_APPEND support (commit
> a23321e766d in xfs xfs-import repository) also checked it to skip
> releasing speculative preallocations, which doesn't make any sense.

I disagree, there was a very good reason for this behaviour:
preventing append-only log files from getting excessively fragmented
because speculative prealloc would get removed on close().

i.e. applications that slowly log messages to append only files
with the pattern open(O_APPEND); write(a single line to the log);
close(); caused worst case file fragmentation because the close()
always removed the speculative prealloc beyond EOF.

The fix for this pessimisitic XFS behaviour is for the application
to use chattr +A (like they would for ext3/4) hence triggering the
existence of XFS_DIFLAG_APPEND and that avoided the removal
speculative delalloc removed when the file is closed. hence the
fragmentation problems went away.

Note that fragmentation issue didn't affect the log writes - it
badly affected log reads because it turned them into worse case
random read workloads instead of sequential reads.

As such, I think the justification for this change is wrong and that
it removes a longstanding feature that prevents severe fragmentation
of append only log files. I think we should be leaving this code as
it currently stands.

-Dave.
-- 
Dave Chinner
david@fromorbit.com