xfs
[Top] [All Lists]

Re: [PATCH] Implement fallocate

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [PATCH] Implement fallocate
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 30 Oct 2007 10:09:06 +0000
Cc: xfs@xxxxxxxxxxx, xfs-dev@xxxxxxx
In-reply-to: <20071029233841.GT995458@xxxxxxx>
References: <20071029233841.GT995458@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.3i
On Tue, Oct 30, 2007 at 10:38:41AM +1100, David Chinner wrote:
> +/*
> + * generic space allocation vector.
> + */

This comment is rather non-sensical.  But I think you can just kill
it as inode operations don't really need comments.

> +STATIC long
> +xfs_vn_fallocate(
> +     struct inode    *inode,
> +     int             mode,
> +     loff_t          offset,
> +     loff_t          len)
> +{
> +     long            error;
> +     loff_t          new_size = 0;
> +     xfs_flock64_t   bf;
> +
> +     /* preallocation on directories not yet supported */
> +     error = -ENODEV;
> +     if (S_ISDIR(inode->i_mode))
> +             goto out_error;
> +
> +     bf.l_whence = 0;
> +     bf.l_start = offset;
> +     bf.l_len = len;
> +
> +     xfs_ilock(XFS_I(inode), XFS_IOLOCK_EXCL);
> +     error = xfs_change_file_space(XFS_I(inode), XFS_IOC_RESVSP, &bf,
> +                                             0, NULL, ATTR_NOLOCK);

please add a 

        struct xfs_inode *ip = XFS_I(inode);

to the beginning of the function and use it subsequently.  That'll make the
fun ction a little more readable.

Otherwise this looks fine to me.


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