Re: [PATCH] Implement fallocate

[Christoph Hellwig <hch@infradead.org>]

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.