xfs
[Top] [All Lists]

Re: [PATCH 02/14] xfs: fix rounding in xfs_free_file_space

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 02/14] xfs: fix rounding in xfs_free_file_space
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 20 May 2013 14:03:03 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1369007481-15185-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1369007481-15185-1-git-send-email-david@xxxxxxxxxxxxx> <1369007481-15185-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6
On 05/19/2013 07:51 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> The offset passed into xfs_free_file_space() needs to be rounded
> down to a certain size, but the rounding mask is built by a 32 bit
> variable. Hence the mask will always mask off the upper 32 bits of
> the offset and lead to incorrect writeback and invalidation ranges.
> 
> This is not actually exposed as a bug because we writeback and
> invalidate from the rounded offset to the end of the file, and hence
> the offset we are actually punching a hole out of will always be
> covered by the code. This needs fixing, however, if we ever want to
> use exact ranges for writeback/invalidation here...
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---

Looks good to me.

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_vnodeops.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 1501f4f..0176bb2 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -1453,7 +1453,7 @@ xfs_free_file_space(
>       xfs_mount_t             *mp;
>       int                     nimap;
>       uint                    resblks;
> -     uint                    rounding;
> +     xfs_off_t               rounding;
>       int                     rt;
>       xfs_fileoff_t           startoffset_fsb;
>       xfs_trans_t             *tp;
> @@ -1482,7 +1482,7 @@ xfs_free_file_space(
>               inode_dio_wait(VFS_I(ip));
>       }
>  
> -     rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> +     rounding = max_t(xfs_off_t, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
>       ioffset = offset & ~(rounding - 1);
>       error = -filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
>                                             ioffset, -1);
> 

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