xfs: flush the range before zero range conversion
PATCH 1/1 xfs: flush the range before zero range conversion
XFS currently discards delalloc blocks within the target range of a zero range request. Unaligned start and end offsets are zeroed through the page cache and the internal, aligned blocks are converted to unwritten extents.
If EOF is page aligned and covered by a delayed allocation extent. The inode size is not updated until I/O completion. If a zero range request discards a delalloc range that covers page aligned EOF as such, the inode size update never occurs. For example:
$ rm -f /mnt/file $ xfs_io -fc "pwrite 0 64k" -c "zero 60k 4k" /mnt/file $ stat -c "%s" /mnt/file 65536 $ umount /mnt $ mount <dev> /mnt $ stat -c "%s" /mnt/file 61440
Update xfs_zero_file_space() to flush the range rather than discard delalloc blocks to ensure that inode size updates occur appropriately.
---
I suppose we could be more clever here and only flush the range in this particular scenario, but I'm not sure if there's a major benefit there. FWIW, this implicitly addresses the indlen==0 assert failures described in the xfs_bmap_del_extent() rfc, but doesn't necessarily mean we shouldn't fix that code IMO.
Brian
fs/xfs/xfs_bmap_util.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
fs/xfs/xfs_bmap_util.c +7 -7
@@ -1394,14 +1394,14 @@ xfs_zero_file_space(if (start_boundary < end_boundary -1 ) {/* -* punch out delayed allocation blocks and the page cache over -* the conversion range +* Writeback the range to ensure any inode size updates due to +* appending writes make it to disk (otherwise we could just +* punch out the delalloc blocks). */ -xfs_ilock (ip,XFS_ILOCK_EXCL ); - error =xfs_bmap_punch_delalloc_range (ip, -XFS_B_TO_FSBT (mp, start_boundary), -XFS_B_TO_FSB (mp, end_boundary - start_boundary)); -xfs_iunlock (ip,XFS_ILOCK_EXCL ); + error =filemap_write_and_wait_range (VFS_I (ip)->i_mapping, + start_boundary, end_boundary -1 ); +if (error) + goto out;truncate_pagecache_range (VFS_I (ip), start_boundary, end_boundary -1 );
Signature · 6 lines
1.8.3.1 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 19 lines · Brian Foster
XFS currently discards delalloc blocks within the target range of a zero range request. Unaligned start and end offsets are zeroed through the page cache and the internal, aligned blocks are converted to unwritten extents.
If EOF is page aligned and covered by a delayed allocation extent. The inode size is not updated until I/O completion. If a zero range request discards a delalloc range that covers page aligned EOF as such, the inode size update never occurs. For example:
$ rm -f /mnt/file $ xfs_io -fc "pwrite 0 64k" -c "zero 60k 4k" /mnt/file $ stat -c "%s" /mnt/file 65536 $ umount /mnt $ mount <dev> /mnt $ stat -c "%s" /mnt/file 61440
Quoted 8 lines
Update
xfs_zero_file_space()to flush the range rather than discard delalloc blocks to ensure that inode size updates occur appropriately.Signed-off-by Brian Foster <[email protected]>---
I suppose we could be more clever here and only flush the range in this particular scenario, but I'm not sure if there's a major benefit there.
Punching the delalloc range rather than flushing the file was done intentionally - this was added primarily for speeding up the zeroing of large VM image files. i.e. it's an extent manipulation operation rather than a data Io operation. Flushing the file defeats the primary reason for the operation existing.
We can easily detect this situation and just zero the last block in the file directly after punching out all the delalloc state. This should happen anyway when the region to be zeroed is not page aligned....
Quoted 3 lines
FWIW, this implicitly addresses the indlen==0 assert failures described in the
xfs_bmap_del_extent()rfc, but doesn't necessarily mean we shouldn't fix that code IMO.
We punch delalloc extents elsewhere, so that still needs fixing.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 35 lines · Dave Chinner
Quoted 19 lines · Brian Foster
XFS currently discards delalloc blocks within the target range of a zero range request. Unaligned start and end offsets are zeroed through the page cache and the internal, aligned blocks are converted to unwritten extents.
If EOF is page aligned and covered by a delayed allocation extent. The inode size is not updated until I/O completion. If a zero range request discards a delalloc range that covers page aligned EOF as such, the inode size update never occurs. For example:
$ rm -f /mnt/file $ xfs_io -fc "pwrite 0 64k" -c "zero 60k 4k" /mnt/file $ stat -c "%s" /mnt/file 65536 $ umount /mnt $ mount <dev> /mnt $ stat -c "%s" /mnt/file 61440Quoted 8 lines
Update
xfs_zero_file_space()to flush the range rather than discard delalloc blocks to ensure that inode size updates occur appropriately.Signed-off-by Brian Foster <[email protected]>---
I suppose we could be more clever here and only flush the range in this particular scenario, but I'm not sure if there's a major benefit there.
Punching the delalloc range rather than flushing the file was done intentionally - this was added primarily for speeding up the zeroing of large VM image files. i.e. it's an extent manipulation operation rather than a data Io operation. Flushing the file defeats the primary reason for the operation existing.
Quoted 5 lines
We can easily detect this situation and just zero the last block in the file directly after punching out all the delalloc state. This should happen anyway when the region to be zeroed is not page aligned....
Hmm, good point. xfs_iozero() goes through page cache. It seems like that should work and it's something we already have to handle in this path. I'll give it a shot.
Brian
Quoted 4 lines
Quoted 3 lines
FWIW, this implicitly addresses the indlen==0 assert failures described in the
xfs_bmap_del_extent()rfc, but doesn't necessarily mean we shouldn't fix that code IMO.
Quoted 13 lines
We punch delalloc extents elsewhere, so that still needs fixing.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs