xfs
[Top] [All Lists]

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

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH V2] fix readahead calculations in xfs_dir2_leaf_getdents()
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 07 Oct 2009 17:24:54 -0500
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, tobias@xxxxxxxxxxxxxxx, xfs mailing list <xfs@xxxxxxxxxxx>
In-reply-to: <1AB9A794DBDDF54A8A81BE2296F7BDFE83AD38@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <1AB9A794DBDDF54A8A81BE2296F7BDFE83AD38@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.23 (Macintosh/20090812)
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@xxxxxx>

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


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