xfs
[Top] [All Lists]

[PATCH] fix readahead calculations in xfs_dir2_leaf_getdents()

To: xfs mailing list <xfs@xxxxxxxxxxx>
Subject: [PATCH] fix readahead calculations in xfs_dir2_leaf_getdents()
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 23 Sep 2009 11:49:22 -0500
Cc: tobias@xxxxxxxxxxxxxxx
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
This is for bug #850,
http://oss.sgi.com/bugzilla/show_bug.cgi?id=850
XFS file system segfaults , repeatedly and 100% reproducable in 2.6.30 , 2.6.31

The above only showed up on a CONFIG_XFS_DEBUG=y kernel, because
xfs_bmapi() ASSERTs that it has been asked for at least one map,
and it was getting 0.

The root cause is that our guesstimated "bufsize" from xfs_file_readdir
was fairly small, and the

                bufsize -= length;

in the loop was going negative - except bufsize is a size_t, so it 
was wrapping to a very large number.

Then when we did
                ra_want = howmany(bufsize + mp->m_dirblksize,
                                  mp->m_sb.sb_blocksize) - 1;

with that very large number, the (int) ra_want was coming out
negative, and a subsequent compare:

                if (1 + ra_want > map_blocks ...

was coming out -true- (negative int compare w/ uint) and we went 
back to xfs_bmapi() for more, even though we did not need more,
and asked for 0 maps, and hit the ASSERT.

We have kind of a type mess here, but just keeping bufsize from 
going negative is probably sufficient to avoid the problem.

I'm open to more tidy or obvious solutions though.  :)

Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
---

diff --git a/fs/xfs/xfs_dir2_leaf.c b/fs/xfs/xfs_dir2_leaf.c
index fa913e4..946562d 100644
--- 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,11 @@ xfs_dir2_leaf_getdents(
                 */
                ptr += length;
                curoff += length;
-               bufsize -= length;
+               /* bufsize may have just been a guess; don't go negative */
+               if (bufsize >= length)
+                       bufsize -= length;
+               else
+                       bufsize = 0;
        }
 
        /*

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