Re: [PATCH V2] fix readahead calculations in xfs_dir2_leaf_getdents()

[Eric Sandeen <sandeen@sandeen.net>]

Alex Elder wrote:
> Eric Sandeen wrote:
>> Christoph Hellwig wrote:
>>> On Fri, Sep 25, 2009 at 02:42:26PM -0500, Eric Sandeen wrote:
>>>> V2: use min() as suggested by Jeff, it's tidier.
>>> I disagree with that, with the cast it looks pretty horrible.
>>> At least use min_t to avoid the case, but what's wrong with:
>>>
>>>> +		/* bufsize may have just been a guess; don't go negative */
>>>> +		bufsize = min((bufsize - length), (size_t)0);
>>> 		bufsize = bufsize - length > 0 ? bufsize - length : 0;
>> ok, that's fine too.
>>
>> I'll pick one.
>>
>>> Anyway, takes this as a
>>>
>>>
>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>>
>>> for any variant.
>>>
>> thanks,
>> -Eric
> 
> I'm going to put in this version:
> 
> 		bufsize = bufsize > length ? bufsize - length : 0;

Fine by me, thanks!

-Eric

> I.e.:
> 
> --- a/fs/xfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/xfs_dir2_leaf.c
> @@ -854,6 +854,7 @@ xfs_dir2_leaf_getdents(
>  			 */
>  			ra_want = howmany(bufsize + mp->m_dirblksize,
>  					  mp->m_sb.sb_blocksize) - 1;
> +			ASSERT(ra_want >= 0);
>  
>  			/*
>  			 * If we don't have as many as we want, and we haven't
> @@ -1088,7 +1089,8 @@ xfs_dir2_leaf_getdents(
>  		 */
>  		ptr += length;
>  		curoff += length;
> -		bufsize -= length;
> +		/* bufsize may have just been a guess; don't go negative */
> +		bufsize = bufsize > length ? bufsize - length : 0;
>  	}
>  
>  	/*
> 
> 					-Alex
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs