xfs
[Top] [All Lists]

Re: [PATCH 02/32 V2] xfs: remove xfs_tosspages

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 02/32 V2] xfs: remove xfs_tosspages
From: Andrew Dahl <adahl@xxxxxxx>
Date: Wed, 14 Nov 2012 12:50:47 -0600
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20121114064247.GC1710@dastard>
References: <1352721264-3700-1-git-send-email-david@xxxxxxxxxxxxx> <1352721264-3700-3-git-send-email-david@xxxxxxxxxxxxx> <20121114064247.GC1710@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121029 Thunderbird/16.0.2
On 11/14/2012 12:42 AM, Dave Chinner wrote:
> xfs: remove xfs_tosspages
> 
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> It's a buggy, unnecessary wrapper that is duplicating
> truncate_pagecache_range().
> 
> When replacing the call in xfs_change_file_space(), also ensure that
> the length being allocated/freed is always positive before making
> any changes. These checks are done in the lower extent manipulation
> functions, too, but we need to do them before any page cache
> operations.
> 
> Reported-by: Andrew Dahl <adahl@xxxxxxx>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---

>       case XFS_IOC_ZERO_RANGE:
>               prealloc_type |= XFS_BMAPI_CONVERT;
> -             xfs_tosspages(ip, startoffset, startoffset + bf->l_len, 0);
> +             end = round_down(startoffset + bf->l_len, PAGE_SIZE) - 1;
> +             if (startoffset > end)

This should be

if (startoffset <= end)

This exact like was in my original patch, though it returned if this was
true. -- Also, it needs to be "or equal to" for the case of passing
[4095,4096] -- after we round down and subtract one, they'll be equal
and the call will zero one byte.

--

I'll follow up with a patch just so Ben can get this pulled today.

Looks great though!

Reviewed-By: Andrew Dahl <adahl@xxxxxxx>

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