[lkml]   [2012]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 2/2] xfs: add FALLOC_FL_ZERO_RANGE to fallocate
    On Tue, Jun 12, 2012 at 05:36:04PM +0200, Paolo Bonzini wrote:
    > This is implemented in the same way as the other fallocate modes. All of
    > them map to XFS ioctls that are implemented by xfs_change_file_space.
    > However, we need to cap the length to the inode size if the user requested

    That's done on purpose. fallocate() explicitly allows preallocation
    beyond EOF and that's what the FALLOC_FL_KEEP_SIZE flag is for - to
    allow both offset and offset+len to lie beyond the current inode
    size and have the preallocation succeed without changing the file

    This is so that we can prevent fragmentation of slow growing
    append-only files like log files - we can preallocate way beyond EOF
    without changing EOF so as the file grows over days and weeks it
    does not fragment.

    Similarly, hole punch needs to be able to punch out such
    preallocated extents beyond EOF if requested, and it definitely must
    not change EOF. So capping/erroring out when offset/offset+len is
    definitely the wrong thing to do when FALLOC_FL_KEEP_SIZE is set.

    > Cc: Dave Chinner <>
    > Signed-off-by: Paolo Bonzini <>
    > ---
    > fs/xfs/xfs_file.c | 36 ++++++++++++++++++++++++------------
    > 1 files changed, 24 insertions(+), 12 deletions(-)
    > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
    > index 9f7ec15..c811cf9 100644
    > --- a/fs/xfs/xfs_file.c
    > +++ b/fs/xfs/xfs_file.c
    > @@ -818,33 +818,45 @@ xfs_file_fallocate(
    > loff_t new_size = 0;
    > xfs_flock64_t bf;
    > xfs_inode_t *ip = XFS_I(inode);
    > - int cmd = XFS_IOC_RESVSP;
    > + int cmd;
    > int attr_flags = XFS_ATTR_NOLOCK;
    > return -EOPNOTSUPP;
    > + BUG_ON((mode & FALLOC_FL_PUNCH_HOLE) && (mode & FALLOC_FL_ZERO_RANGE));

    Never put BUG_ON() or BUG() in XFS code that can return an error.
    Return EINVAL if we chose not to support it, and if it's really
    something we consider bad, emit a warning to syslog (i.e.
    xfs_warn()) and potentially add a ASSERT() case so that debug
    kernels will trip over it. Nobody should be panicing a production
    system just because a user supplied a set of incorrect syscall


    Dave Chinner

     \ /
      Last update: 2012-06-13 05:01    [W:0.029 / U:0.068 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site