lkml.org 
[lkml]   [2009]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch -mmotm] mm: invoke oom killer for __GFP_NOFAIL
On Sat, 9 May 2009 15:46:39 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:

> The oom killer must be invoked regardless of the order if the allocation
> is __GFP_NOFAIL, otherwise it will loop forever when reclaim fails to
> free some memory.

Sigh. We're supposed to be deleting __GFP_NOFAIL. I added it as a way
of easily finding lame error-handling-challenged callers which need to
be fixed up. So of course we went and added lots more callers.

y:/usr/src/linux-2.6.30-rc5> grep -rl GFP_NOFAIL .
./arch/x86/xen/mmu.c
./arch/sparc/kernel/mdesc.c
./mm/page_alloc.c
./mm/failslab.c
./block/cfq-iosched.c
./fs/bio-integrity.c
./fs/ntfs/ChangeLog
./fs/ntfs/malloc.h
./fs/reiserfs/journal.c
./fs/gfs2/meta_io.c
./fs/gfs2/rgrp.c
./fs/gfs2/dir.c
./fs/gfs2/log.c
./fs/jbd/transaction.c
./fs/jbd/journal.c
./fs/jbd2/transaction.c
./fs/jbd2/journal.c
./drivers/net/cxgb3/cxgb3_main.c
./drivers/net/cxgb3/cxgb3_offload.c
./include/linux/slab.h
./include/linux/gfp.h

JBD (and hence JBD2) are the original sinners.

That net driver should be taught to just handle the allocation failure,
please.


It's super-uber-bad to be using __GFP_NOFAIL in an IO scheduler! But maybe
that's just a brainfart:

/*
* Inform the allocator of the fact that we will
* just repeat this allocation if it fails, to allow
* the allocator to do whatever it needs to attempt to
* free memory.
*/
If "we will just repeat this allocation" means what it says then we
should use __GFP_NORETRY here, then retry the allocation if it failed.
But a) this risks getting stuck in a hot loop in CFQ and b) we really
really don't want to be looping infinitely for memory relcaim down in
the guts of the block layer!

From my reading, this function is called from get_request_wait(), via

rq = get_request(q, rw_flags, bio, GFP_NOIO);

so we can't even do pageout here.

Jens, this all looks quite risky.


\
 
 \ /
  Last update: 2009-05-11 22:49    [from the cache]
©2003-2011 Jasper Spaans