xfs
[Top] [All Lists]

Re: [Bug 335] New: XFS Oops with 2.6.7-rc3

To: linux-xfs@xxxxxxxxxxx
Subject: Re: [Bug 335] New: XFS Oops with 2.6.7-rc3
From: Andi Kleen <ak@xxxxxx>
Date: Wed, 09 Jun 2004 13:01:45 +0200
In-reply-to: <200406090934.i599YaKP022234@xxxxxxxxxxx> (bugzilla-daemon@xxxxxxxxxxx's message of "Wed, 9 Jun 2004 02:34:36 -0700")
References: <200406090934.i599YaKP022234@xxxxxxxxxxx>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Gnus/5.110003 (No Gnus v0.3) Emacs/21.2 (gnu/linux)
bugzilla-daemon@xxxxxxxxxxx writes:

> http://oss.sgi.com/bugzilla/show_bug.cgi?id=335
>
>            Summary: XFS Oops with 2.6.7-rc3
>            Product: Linux XFS
>            Version: Current
>           Platform: All
>         OS/Version: Linux
>             Status: NEW
>           Severity: normal
>           Priority: High
>          Component: xfsdump
>         AssignedTo: xfs-master@xxxxxxxxxxx
>         ReportedBy: evilninja@xxxxxxx
>
>
> during copying with MidnightCommander (curses based filemanager) an Oops
> showed up on the console, but the machine is still working. no lockups,
> i can still access/umount/mount the fs. xfs_check did not reveal any
> errors. the machine is a p3/500, debian/unstable (xfsprogs 2.6.11-1),
> glibc 2.3.2.ds1-12, mount/umount utils from loop-aes-utils package, if
> this matters. more info available, of course.
>
> mc: page allocation failure. order:5, mode:0x50
[...]

> Unable to handle kernel NULL pointer dereference at virtual address 00000000
[...]

This patch fixes it. The problem is that XFS expects __GFP_NOFAIL 
to never return NULL, but it returns NULL anyways when PF_MEMALLOC
is set for the current process. This patch just readds looping 
in the XFS allocator.

In addition it stops recursions inside PF_MEMALLOC.

-Andi

diff -u linux/fs/xfs/linux/kmem.h-o linux/fs/xfs/linux/kmem.h
--- linux/fs/xfs/linux/kmem.h-o 2004-05-24 14:01:42.000000000 +0200
+++ linux/fs/xfs/linux/kmem.h   2004-05-24 14:12:14.000000000 +0200
@@ -56,7 +56,8 @@
 
 typedef unsigned long xfs_pflags_t;
 
-#define PFLAGS_TEST_FSTRANS()           (current->flags & PF_FSTRANS)
+#define PFLAGS_NO_RECURSION() \
+       (current->flags & (PF_FSTRANS|PF_MEMALLOC|PF_MEMDIE))
 
 /* these could be nested, so we save state */
 #define PFLAGS_SET_FSTRANS(STATEP) do {        \
@@ -98,7 +99,7 @@
                lflags = GFP_KERNEL;
 
                /* avoid recusive callbacks to filesystem during transactions */
-               if (PFLAGS_TEST_FSTRANS() || (flags & KM_NOFS))
+               if (PFLAGS_NO_RECURSION() || (flags & KM_NOFS))
                        lflags &= ~__GFP_FS;
 
                if (!(flags & KM_MAYFAIL))
@@ -159,7 +160,20 @@
 static __inline void *
 kmem_zone_alloc(kmem_zone_t *zone, int flags)
 {
-       return kmem_cache_alloc(zone, kmem_flags_convert(flags));
+       /* __GFP_NOFAIL doesn't work properly when PF_MEMALLOC is already 
+          set, and this can happen in XFS when it is called in low memory
+          situations to free up some cached data. Loop on our own.
+
+          This has some deadlock potential in extreme OOM cases.
+
+          It would be better to use mempools for the allocations that 
+          are used from the low memory paths (iput, writepage); like
+          for the transactions. */
+       void *p;
+       do {
+               p = kmem_cache_alloc(zone, kmem_flags_convert(flags));
+       } while (!p && !(flags & KM_MAYFAIL));
+       return p;
 }
 
 static __inline void *
diff -u linux/fs/xfs/linux/xfs_aops.c-o linux/fs/xfs/linux/xfs_aops.c
--- linux/fs/xfs/linux/xfs_aops.c-o     2004-05-25 02:52:54.211070280 +0200
+++ linux/fs/xfs/linux/xfs_aops.c       2004-05-25 02:53:41.662856512 +0200
@@ -1168,7 +1168,7 @@
         * then mark the page dirty again and leave the page
         * as is.
         */
-       if (PFLAGS_TEST_FSTRANS() && need_trans)
+       if (PFLAGS_NO_RECURSION() && need_trans)
                goto out_fail;
 
        /*
@@ -1242,7 +1242,7 @@
        /* If we are already inside a transaction or the thread cannot
         * do I/O, we cannot release this page.
         */
-       if (PFLAGS_TEST_FSTRANS())
+       if (PFLAGS_NO_RECURSION())
                return 0;
 
        /*


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