Important regression with XFS update for 2.6.24-rc6

Hello,

As a follow-up to <http://marc.info/?l=linux-kernel&m=119796120524618&w=2>
(LKML seems down right now so I am not linking to it), I have detected an important problem with these two patches: after applying them by hand (downloaded them raw from SGI's gitweb) on top of 2.6.24-rc5-git5 (they have not yet been pulled into mainline by Linux as of this morning) for testing purposes, I noticed upon reboot that "ls -l" on directories with many files and subdirectories (around 5000 entries) takes several hundreds of MB in RAM and then dies with "memory exhausted" error.

I also noticed that ldconfig takes a lot of time to complete, and firefox seems also to eat much more memory than usual. Reverting the two patches (going back to vanilla rc5-git5) makes these problems go away. I am not able to test right now if only one of the patches is bogus or if both of them are concerned.

As the symptoms are easy to reproduce, I guess this is some kind of brown paper bag bug and will be easy for XFS experts to spot.

Ok. I haven't noticed anything wrong with directories up to about 250,000 files in the last few days. The ls -l I just did on a directory with 15000 entries (btree format) used about 5MB of RAM. extent format directories appear to work fine as well (tested 500 entries).

Can you:

a) isolate the problem to one patch or the other. My guess would be the directory mod, but.....

b) show your working ;)

- what platform (i386, x86_64, etc)

- what debug options

- commands and output that shows the problem

- strace of ls -l going bad
- xfs_info from filesystem in question

Quoted 5 lines · David Chinner

Ok. I haven't noticed anything wrong with directories up to about 250,000 files in the last few days. The ls -l I just did on a directory with 15000 entries (btree format) used about 5MB of RAM. extent format directories appear to work fine as well (tested 500 entries).

Ok, nice to know the problem is not so frequent.

I have put the files at http://damien.wyart.free.fr/xfs/

strace_xfs_problem.1.gz and strace_xfs_problem.2.gz have been created with the problematic kernel, and are quite bigger than strace_xfs_problem.normal.gz, which has been created with the vanilla rc5-git5. There is also xfs_info.

Quoted 8 lines · Damien Wyart

Quoted 5 lines · David Chinner

Ok. I haven't noticed anything wrong with directories up to about 250,000 files in the last few days. The ls -l I just did on a directory with 15000 entries (btree format) used about 5MB of RAM. extent format directories appear to work fine as well (tested 500 entries).

Ok, nice to know the problem is not so frequent.

.....

Quoted 6 lines

I have put the files at http://damien.wyart.free.fr/xfs/

strace_xfs_problem.1.gz and strace_xfs_problem.2.gz have been created with the problematic kernel, and are quite bigger than strace_xfs_problem.normal.gz, which has been created with the vanilla rc5-git5. There is also xfs_info.

Looks like several getdents() through the directory the getdents() call starts outputting the first files again. It gets to a certain point and always goes back to the beginning. However, it appears to get to the end eventually (without ever getting past the bad offset).

Can someone pass me a brown paper bag, please?

That also explains why we haven't seen it - it requires the user buffer to fill on the first entry of a backing buffer and so it is largely dependent on the pattern of name lengths, page size and filesystem block size aligning just right to trigger the problem.

Can you test this patch, Damien?

Quoted 1 line · David Chinner

Can someone pass me a brown paper bag, please?

My first impression on this bug was not so wrong, after all ;-)

Quoted 4 lines

That also explains why we haven't seen it - it requires the user buffer to fill on the first entry of a backing buffer and so it is largely dependent on the pattern of name lengths, page size and filesystem block size aligning just right to trigger the problem.

I guess I was lucky to trigger it quite easily...

Quoted 1 line

Can you test this patch, Damien?

Works fine, all the bad symptoms have disappeared and strace output is normal.

So you can add:

Tested-by Damien Wyart <[email protected]>

Quoted 19 lines · David Chinner

Quoted 8 lines · Damien Wyart

Quoted 5 lines · David Chinner

Ok. I haven't noticed anything wrong with directories up to about 250,000 files in the last few days. The ls -l I just did on a directory with 15000 entries (btree format) used about 5MB of RAM. extent format directories appear to work fine as well (tested 500 entries).

Ok, nice to know the problem is not so frequent.

.....

Quoted 6 lines

I have put the files at http://damien.wyart.free.fr/xfs/

strace_xfs_problem.1.gz and strace_xfs_problem.2.gz have been created with the problematic kernel, and are quite bigger than strace_xfs_problem.normal.gz, which has been created with the vanilla rc5-git5. There is also xfs_info.

Quoted 4 lines

Looks like several getdents() through the directory the getdents() call starts outputting the first files again. It gets to a certain point and always goes back to the beginning. However, it appears to get to the end eventually (without ever getting past the bad offset).

UML and a bunch of printk's to the rescue.

So we went back to double buffering, which then screwed up the d_off of the dirents. I changed the temporary dirents to point to the current offset so that filldir got what it expected when filling the user buffer.

Except it appears that it I didn't to initialise the current offset for the first dirent read from the temporary buffer so filldir occasionally got an uninitialised offset. Can someone pass me a brown paper bag, please?

In my local testing, more often than not, that uninitialised offset reads as zero which is where the looping comes from. Sometimes it points off into wacko-land, which is probably how we eventually get the looping terminating before you run out of memory.

That also explains why we haven't seen it - it requires the user buffer to fill on the first entry of a backing buffer and so it is largely dependent on the pattern of name lengths, page size and filesystem block size aligning just right to trigger the problem.

Can you test this patch, Damien?

Cheers,

Dave.

Signature · 10 lines

Dave Chinner
Principal Engineer
SGI Australian Software Group

---
 fs/xfs/linux-2.6/xfs_file.c |    1 +
 1 file changed, 1 insertion(+)

Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c
===================================================================

fs/xfs/linux-2.6/xfs_file.c +1 -0

@@ -348,6 +348,7 @@ xfs_file_readdir(
 
 		size = buf.used;
 		de = (struct hack_dirent *)buf.dirent;
+		curr_offset = de->offset /* & 0x7fffffff */;
 		while (size > 0) {
 			if (filldir(dirent, de->name, de->namlen,
 					curr_offset & 0x7fffffff,

This has run through several iterations of xfsqa now, and it fixes the reported problem, so can I get a review?

Cheers,

Dave.

Quoted 68 lines · David Chinner

Quoted 12 lines · David Chinner

Quoted 7 lines · Damien Wyart

Quoted 5 lines · David Chinner

Ok. I haven't noticed anything wrong with directories up to about 250,000 files in the last few days. The ls -l I just did on a directory with 15000 entries (btree format) used about 5MB of RAM. extent format directories appear to work fine as well (tested 500 entries).

Ok, nice to know the problem is not so frequent.

.....

Quoted 2 lines

I have put the files at http://damien.wyart.free.fr/xfs/

Quoted 8 lines

Quoted 4 lines

strace_xfs_problem.1.gz and strace_xfs_problem.2.gz have been created with the problematic kernel, and are quite bigger than strace_xfs_problem.normal.gz, which has been created with the vanilla rc5-git5. There is also xfs_info.

Looks like several getdents() through the directory the getdents() call starts outputting the first files again. It gets to a certain point and always goes back to the beginning. However, it appears to get to the end eventually (without ever getting past the bad offset).

UML and a bunch of printk's to the rescue.

So we went back to double buffering, which then screwed up the d_off of the dirents. I changed the temporary dirents to point to the current offset so that filldir got what it expected when filling the user buffer.

Except it appears that it I didn't to initialise the current offset for the first dirent read from the temporary buffer so filldir occasionally got an uninitialised offset. Can someone pass me a brown paper bag, please?

In my local testing, more often than not, that uninitialised offset reads as zero which is where the looping comes from. Sometimes it points off into wacko-land, which is probably how we eventually get the looping terminating before you run out of memory.

That also explains why we haven't seen it - it requires the user buffer to fill on the first entry of a backing buffer and so it is largely dependent on the pattern of name lengths, page size and filesystem block size aligning just right to trigger the problem.

Can you test this patch, Damien?

Cheers,

Dave.

Signature · 10 lines

Dave Chinner
Principal Engineer
SGI Australian Software Group

---
 fs/xfs/linux-2.6/xfs_file.c |    1 +
 1 file changed, 1 insertion(+)

Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_file.c
===================================================================

fs/xfs/linux-2.6/xfs_file.c +1 -0

@@ -348,6 +348,7 @@ xfs_file_readdir(
 
 		size = buf.used;
 		de = (struct hack_dirent *)buf.dirent;
+		curr_offset = de->offset /* & 0x7fffffff */;
 		while (size > 0) {
 			if (filldir(dirent, de->name, de->namlen,
 					curr_offset & 0x7fffffff,