On Tue, Aug 27, 2024 at 08:50:50AM +0200, Christoph Hellwig wrote: > Refactor xfs_file_fallocate into separate helpers for each mode, > two factors for i_size handling and a single switch statement over the > supported modes. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > fs/xfs/xfs_file.c | 330 +++++++++++++++++++++++++++++----------------- > 1 file changed, 208 insertions(+), 122 deletions(-) Much nicer. :) And it made an existing issue in the code quite obvious, too: > +/* > + * Punch a hole and prealloc the range. We use a hole punch rather than > + * unwritten extent conversion for two reasons: > + * > + * 1.) Hole punch handles partial block zeroing for us. > + * 2.) If prealloc returns ENOSPC, the file range is still zero-valued by > + * virtue of the hole punch. > + */ > +static int > +xfs_falloc_zero_range( > + struct file *file, > + int mode, > + loff_t offset, > + loff_t len) > +{ > + struct inode *inode = file_inode(file); > + unsigned int blksize = i_blocksize(inode); > + loff_t new_size = 0; > + int error; > + > + trace_xfs_zero_file_space(XFS_I(inode)); > + > + error = xfs_falloc_newsize(file, mode, offset, len, &new_size); > + if (error) > + return error; > + > + error = xfs_free_file_space(XFS_I(inode), offset, len); > + if (error) > + return error; > + > + len = round_up(offset + len, blksize) - round_down(offset, blksize); > + offset = round_down(offset, blksize); > + error = xfs_alloc_file_space(XFS_I(inode), offset, len); > + if (error) > + return error; > + return xfs_falloc_setsize(file, new_size); > +} Our zeroing operation always does preallocation, but.... > +static int > +xfs_falloc_allocate_range( > + struct file *file, > + int mode, > + loff_t offset, > + loff_t len) > +{ > + struct inode *inode = file_inode(file); > + loff_t new_size = 0; > + int error; > + > + /* > + * If always_cow mode we can't use preallocations and thus should not > + * create them. > + */ > + if (xfs_is_always_cow_inode(XFS_I(inode))) > + return -EOPNOTSUPP; ... our preallocation operation always returns -EOPNOTSUPP for COW mode. Should the zeroing code also have this COW mode check in it after the hole punch has run so we don't do unnecessary prealloc there? -Dave. -- Dave Chinner david@xxxxxxxxxxxxx