Re: [REPOST] [PATCH] fs/xfs/linux-2.6/kmem.c

[Nathan Scott <nathans@xxxxxxx>]

Hi Mike,

On Thu, Nov 25, 2004 at 12:35:06PM +0000, Miquel van Smoorenburg wrote:
> 
> OK, did that. This time I kept as much of the code intact as possible.

Thanks.

> Things I changed:
> 
> - in linux-2.6/kmem.c:
>  o add __GFP_NOWARN to gfp flags so that kmalloc/vmalloc don't
>    spew warnings when we're in a loop
>  o added call to blk_congestion_wait() at the end of the loop
>  o made error printk a bit clearer

Looks good.

> - in linux-2.6/xfs_buf.c
>  o add __GFP_NOWARN to gfp flags so that alloc_pages() doesn't
>    spew warnings when we're in a loop
>  o added call to blk_congestion_wait() at the end of the loop
>  o made error printk a bit clearer
>  o since there was already a 10 ms delay in there, if
>    blk_congestion_wait() waited for less than 10 ms make sure
>    we wait for the remaining amount of time (which is 20 ms now).

Looks good.

>  I'm not sure if we actually need the sleep-at-least-10-ms code,
>  but I'm not sure what it does and at least this way, actual
>  behaviour isn't changed much.

That was there to give the daemon a bit of time to wakeup and
start writeout on delwrite buffers (which will clean pages,
and help with subsequent page allocation attempts) -- it looks
safe to now drop that timeout piece altogether; we'll quickly
initiate I/O and then be held off inside blk_congestion_wait,
either right away or after the next failed find_or_create_page.

> Patch has been tested on a busy usenet newsserver.

I tweaked a couple of things, esp. keeping the uses of GFP flags
together in kmem.h and xfs_buf.c and dropped the timeout.  How
does the following patch fare for you?  (could you also send me
a Signed-off-by: line for when the final patch is merged in?)

thanks!

-- 
Nathan


Index: xfs-linux/linux-2.6/xfs_buf.c
===================================================================
--- xfs-linux.orig/linux-2.6/xfs_buf.c
+++ xfs-linux/linux-2.6/xfs_buf.c
@@ -53,13 +53,10 @@
 #include <linux/workqueue.h>
 #include <linux/suspend.h>
 #include <linux/percpu.h>
+#include <linux/blkdev.h>
 
 #include "xfs_linux.h"
 
-#ifndef GFP_READAHEAD
-#define GFP_READAHEAD  (__GFP_NOWARN|__GFP_NORETRY)
-#endif
-
 /*
  * File wide globals
  */
@@ -118,8 +115,8 @@
  */
 
 #define pb_to_gfp(flags) \
-       (((flags) & PBF_READ_AHEAD) ? GFP_READAHEAD : \
-        ((flags) & PBF_DONT_BLOCK) ? GFP_NOFS : GFP_KERNEL)
+       ((((flags) & PBF_READ_AHEAD) ? __GFP_NORETRY : \
+         ((flags) & PBF_DONT_BLOCK) ? GFP_NOFS : GFP_KERNEL) | __GFP_NOWARN)
 
 #define pb_to_km(flags) \
         (((flags) & PBF_DONT_BLOCK) ? KM_NOFS : KM_SLEEP)
@@ -388,13 +385,13 @@
                         */
                        if (!(++retries % 100))
                                printk(KERN_ERR
-                                       "possible deadlock in %s (mode:0x%x)\n",
+                                       "XFS: possible memory allocation "
+                                       "deadlock in %s (mode:0x%x)\n",
                                        __FUNCTION__, gfp_mask);
 
                        XFS_STATS_INC(pb_page_retries);
                        pagebuf_daemon_wakeup(0, gfp_mask);
-                       set_current_state(TASK_UNINTERRUPTIBLE);
-                       schedule_timeout(10);
+                       blk_congestion_wait(WRITE, HZ/50);
                        goto retry;
                }
 
Index: xfs-linux/linux-2.6/kmem.c
===================================================================
--- xfs-linux.orig/linux-2.6/kmem.c
+++ xfs-linux/linux-2.6/kmem.c
@@ -35,6 +35,7 @@
 #include <linux/vmalloc.h>
 #include <linux/highmem.h>
 #include <linux/swap.h>
+#include <linux/blkdev.h>
 
 #include "time.h"
 #include "kmem.h"
@@ -46,7 +47,8 @@
 void *
 kmem_alloc(size_t size, int flags)
 {
-       int     retries = 0, lflags = kmem_flags_convert(flags);
+       int     retries = 0;
+       int     lflags = kmem_flags_convert(flags);
        void    *ptr;
 
        do {
@@ -57,8 +59,10 @@
                if (ptr || (flags & (KM_MAYFAIL|KM_NOSLEEP)))
                        return ptr;
                if (!(++retries % 100))
-                       printk(KERN_ERR "possible deadlock in %s (mode:0x%x)\n",
+                       printk(KERN_ERR "XFS: possible memory allocation "
+                                       "deadlock in %s (mode:0x%x)\n",
                                        __FUNCTION__, lflags);
+               blk_congestion_wait(WRITE, HZ/50);
        } while (1);
 }
 
@@ -102,7 +106,8 @@
 void *
 kmem_zone_alloc(kmem_zone_t *zone, int flags)
 {
-       int     retries = 0, lflags = kmem_flags_convert(flags);
+       int     retries = 0;
+       int     lflags = kmem_flags_convert(flags);
        void    *ptr;
 
        do {
@@ -110,8 +115,10 @@
                if (ptr || (flags & (KM_MAYFAIL|KM_NOSLEEP)))
                        return ptr;
                if (!(++retries % 100))
-                       printk(KERN_ERR "possible deadlock in %s (mode:0x%x)\n",
+                       printk(KERN_ERR "XFS: possible memory allocation "
+                                       "deadlock in %s (mode:0x%x)\n",
                                        __FUNCTION__, lflags);
+               blk_congestion_wait(WRITE, HZ/50);
        } while (1);
 }
 
Index: xfs-linux/linux-2.6/kmem.h
===================================================================
--- xfs-linux.orig/linux-2.6/kmem.h
+++ xfs-linux/linux-2.6/kmem.h
@@ -83,7 +83,7 @@
 
 static __inline unsigned int kmem_flags_convert(int flags)
 {
-       int lflags;
+       int     lflags = __GFP_NOWARN;  /* we'll report deadlocks, if needed */
 
 #ifdef DEBUG
        if (unlikely(flags & ~(KM_SLEEP|KM_NOSLEEP|KM_NOFS|KM_MAYFAIL))) {