GFP_NOFS broken
Subsystems (6)
While I was travelling disconnected from the internet the last week (I will buy a GPRS phone soon ;) I also spotted and fixed the __GFP_BUFFER deadlock that I was triggering on my 128m laptop while browsing large email folders on top of crypto loop:
include/linux/mm.h renamed from include/linux/mm.h.~1~ +1 -2
@@ -538,9 +538,8 @@#define __GFP_WAIT 0x10#define __GFP_HIGH 0x20#define __GFP_IO 0x40 -#define __GFP_BUFFER 0x80 -#define GFP_BUFFER (__GFP_HIGH | __GFP_WAIT | __GFP_BUFFER) +#define GFP_BUFFER (__GFP_HIGH | __GFP_WAIT)#define GFP_ATOMIC (__GFP_HIGH)#define GFP_USER ( __GFP_WAIT | __GFP_IO)#define GFP_HIGHUSER ( __GFP_WAIT | __GFP_IO | __GFP_HIGHMEM)
include/linux/slab.h renamed from include/linux/slab.h.~1~ +1 -1
@@ -22,7 +22,7 @@#define SLAB_NFS GFP_NFS#define SLAB_DMA GFP_DMA -#define SLAB_LEVEL_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO|__GFP_BUFFER) +#define SLAB_LEVEL_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO)#define SLAB_NO_GROW 0x00001000UL/* don't grow a cache */ /* flags to pass to kmem_cache_create().
mm/vmscan.c renamed from mm/vmscan.c.~1~ +1 -2
@@ -425,7 +425,6 @@*/ #define MAX_LAUNDER (4 * (1 << page_cluster))#define CAN_DO_IO (gfp_mask & __GFP_IO) -#define CAN_DO_BUFFERS (gfp_mask & __GFP_BUFFER)int page_launder (int gfp_mask,int sync) {int launder_loop, maxscan, cleaned_pages, maxlaunder;
@@ -613,7 +612,7 @@* loads, flush out the dirty pages before we have to wait on * IO. */ -if ((CAN_DO_IO ||CAN_DO_BUFFERS ) && !launder_loop &&free_shortage ()) { +if (CAN_DO_IO && !launder_loop &&free_shortage ()) { launder_loop =1 ;/* If we cleaned pages, never do synchronous IO. */ if (cleaned_pages)
However the more finegrined fix that I found in 2.4.7pre2 and 2.4.6 seems broken unlike my above one.
getblk called from the fs calls grow_buffers with GFP_NOFS which doesn't inhibit shrink_dcache_memory to re-enter the fs in prune_dcache because __GFP_IO is set. And it will deadlock as usual when shrink_dcache reenter the fs that way.
This should cure the deadlock (against 2.4.7pre2) but beware it's untested.
fs/dcache.c renamed from fs/dcache.c.~1~ +1 -1
@@ -566,7 +566,7 @@* We should make sure we don't hold the superblock lock over * block allocations, but for now: */ -if (!(gfp_mask & __GFP_IO)) +if (!(gfp_mask & __GFP_FS))return ;if (priority)
fs/inode.c renamed from fs/inode.c.~1~ +1 -1
@@ -696,7 +696,7 @@* want to recurse into the FS that called us * in clear_inode() and friends.. */ -if (!(gfp_mask & __GFP_IO)) +if (!(gfp_mask & __GFP_FS))return ;if (priority)
Secondly this cleanups a bit in page_launder. If we are allowed to enter the FS we can certainly also do I/O.
mm/vmscan.c renamed from mm/vmscan.c.~1~ +1 -1
@@ -612,7 +612,7 @@* loads, flush out the dirty pages before we have to wait on * IO. */ -if ((CAN_DO_IO ||CAN_DO_FS ) && !launder_loop &&free_shortage ()) { +if (CAN_DO_IO && !launder_loop &&free_shortage ()) { launder_loop =1 ;/* If we cleaned pages, never do synchronous IO. */ if (cleaned_pages)
I will include them in my next tree as it is at least certainly better than the previous code.
Andrea
Quoted 5 lines · Andrea Arcangeli
getblk called from the fs calls
grow_bufferswithGFP_NOFSwhich doesn't inhibitshrink_dcache_memoryto re-enter the fs inprune_dcachebecause__GFP_IOis set. And it will deadlock as usual whenshrink_dcachereenter the fs that way.
Good catch. I renamed GFP_BUFFER on purpose to make sure that nobody relied on the old semantics, but I didn't think of the low-level __GFP_IO one which also changed semantics.
A quick "grep" indicates that you seem to have found the only two that were left. Thanks.
Quoted 2 lines
Secondly this cleanups a bit in
page_launder. If we are allowed to enter the FS we can certainly also do I/O.
Yup.
Linus