xfs
[Top] [All Lists]

Re: [PATCH 4/5] xfs: simplify the fallocate path

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/5] xfs: simplify the fallocate path
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Fri, 25 Jan 2013 16:35:26 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20121208121006.286014845@xxxxxxxxxxxxxxxxxxxxxx>
References: <20121208120812.755863148@xxxxxxxxxxxxxxxxxxxxxx> <20121208121006.286014845@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 12/08/12 06:08, Christoph Hellwig wrote:
Call xfs_alloc_file_space or xfs_free_file_space directly from
xfs_file_fallocate instead of going through xfs_change_file_space.

This simplified the code by removing the unessecary marshalling of the
arguments into an xfs_flock64_t structure and allows removing checks that
are already done in the VFS code.

Signed-off-by: Christoph Hellwig<hch@xxxxxx>

---
  fs/xfs/xfs_file.c     |   81 
++++++++++++++++++++++++++++++++------------------
  fs/xfs/xfs_vnodeops.c |   39 ++----------------------
  fs/xfs/xfs_vnodeops.h |    3 +
  3 files changed, 60 insertions(+), 63 deletions(-)

patched code looks like:


STATIC long
xfs_file_fallocate(
        struct file             *file,
        int                     mode,
        loff_t                  offset,
        loff_t                  len)
{
        struct inode            *inode = file->f_path.dentry->d_inode;
        struct xfs_inode        *ip = XFS_I(inode);
        struct xfs_trans        *tp;
        bool                    setprealloc = false;
        long                    error;
        loff_t                  new_size = 0;

        if (!S_ISREG(inode->i_mode))
                return -EINVAL;

        if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
                return -EOPNOTSUPP;

        xfs_ilock(ip, XFS_IOLOCK_EXCL);
        if (mode & FALLOC_FL_PUNCH_HOLE) {
                error = xfs_free_file_space(ip, offset, len);
                if (error)
                        goto out_unlock;
        } else {
           ^^^^^^^^^   vvvvvvvvvvvvvvvv
                if (!(mode & FALLOC_FL_KEEP_SIZE) &&
                    offset + len > i_size_read(inode)) {
                        new_size = offset + len;
                        error = -inode_newsize_ok(inode, new_size);
                        if (error)
                                goto out_unlock;
                }

Since there is only the FALLOC_FL_KEEP_SIZE or FALLOC_FL_PUNCH_HOLE
bit set and the if statement makes them mutually exclusive, the block
highlighted should be for the FALLOC_FL_KEEP_SIZE and shouldn't the
comparison would never be true here? Shouldn't it be for the
FALLOC_FL_PUNCH_HOLE block?

--Mark.

<Prev in Thread] Current Thread [Next in Thread>