xfs
[Top] [All Lists]

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

To: "Eric Sandeen" <sandeen@xxxxxxxxxxx>, "Christoph Hellwig" <hch@xxxxxxxxxxxxx>
Subject: RE: [PATCH V2] fix readahead calculations in xfs_dir2_leaf_getdents()
From: "Alex Elder" <aelder@xxxxxxx>
Date: Wed, 7 Oct 2009 17:22:26 -0500
Cc: <tobias@xxxxxxxxxxxxxxx>, "xfs mailing list" <xfs@xxxxxxxxxxx>
In-reply-to: <4ABE577E.8060303@xxxxxxxxxxx>
Thread-index: Aco+1I+juuZ1yqbCQN24ujEsyblU6wIx0W2A
Thread-topic: [PATCH V2] fix readahead calculations in xfs_dir2_leaf_getdents()
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@xxxxxx>
>> 
>> for any variant.
>> 
> 
> thanks,
> -Eric

I'm going to put in this version:

                bufsize = bufsize > length ? bufsize - length : 0;

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

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