On 11/21/2012 02:05 AM, Dave Chinner wrote:
...
>
> [ Here's a tip for the future: anything that changes allocation
> corner cases needs to be run through the entire of xfstests suite
> because they have a nasty habit of causing secondary problems.... ]
>
Makes sense -- I'll keep that in mind for the future. (Thanks!)
...
> +
> +STATIC int
> +xfs_zero_file_space(
> + struct xfs_inode *ip,
> + xfs_off_t offset,
> + xfs_off_t len,
> + int attr_flags)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + uint rounding;
> + xfs_off_t start;
> + xfs_off_t end;
> + int error;
> +
> + rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
Let's say rounding is 4K
> +
> + /* round the range iof extents we are going to convert inwards */
> + start = round_up(offset, rounding);
> + end = round_down(offset + len, rounding);
Now, let's say we pass in (4K-1) for the offset and (4K-1).
Then start would be 4K and the end would be 4K, right?
> +
> + ASSERT(start >= offset);
> + ASSERT(end <= offset + len);
These are both true, so this is good.
> +
> + if (!(attr_flags & XFS_ATTR_NOLOCK))
> + xfs_ilock(ip, XFS_IOLOCK_EXCL);
> +
> + if (start < end - 1) {
This is false, as expected.
> + /* punch out the page cache over the conversion range */
> + truncate_pagecache_range(VFS_I(ip), start, end - 1);
> + /* convert the blocks */
> + error = xfs_alloc_file_space(ip, start, end - start - 1,
> + XFS_BMAPI_PREALLOC | XFS_BMAPI_CONVERT,
> + attr_flags);
> + if (error)
> + goto out_unlock;
> + } else {
> + /* it's a sub-rounding range */
> + ASSERT(offset + len <= rounding);
This is false. (8K - 2) <= 4K -- Not so good.
Maybe (2*rounding) would be better, as offset + len could never be
greater than 2rounding (but can be greater than 1rounding). Or removing
this assert altogether.
> + error = xfs_iozero(ip, offset, len);
> + goto out_unlock;
> + }
> +
> + /* now we've handled the interior of the range, handle the edges */
> + if (start != offset)
> + error = xfs_iozero(ip, offset, start - offset);
> + if (!error && end != offset + len)
> + error = xfs_iozero(ip, end, offset + len - end);
This looks good.
> +
> +out_unlock:
> + if (!(attr_flags & XFS_ATTR_NOLOCK))
> + xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> + return error;
...
Beyond that, I think it all looks good and like what you've done!
Thanks,
Andrew
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
