Re: upcoming kerneloops.org item: get_page_from_freelist

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 24 Jun 2009 13:40:11 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Wed, 24 Jun 2009, Linus Torvalds wrote:
> > On Wed, 24 Jun 2009, Andrew Morton wrote:
> > > 
> > > If the caller gets oom-killed, the allocation attempt fails.  Callers need
> > > to handle that.
> > 
> > I actually disagree. I think we should just admit that we can always free 
> > up enough space to get a few pages, in order to then oom-kill things.
> 
> Btw, if you want to change the WARN_ON() to warn when you're in the 
> "allocate in order to free memory" recursive case, then I'd have no issues 
> with that.
> 
> In fact, in that case it probably shouldn't even be conditional on the 
> order. 
> 
> So a
> 
> 	WARN_ON_ONCE((p->flags & PF_MEMALLOC) && (gfpmask & __GFP_NOFAIL));
> 
> actually makes tons of sense. 

I suspect that warning will trigger.

alloc_pages
-> ...
  -> pageout
    -> ...
      -> get_request
        -> blk_alloc_request
          -> elv_set_request
            -> cfq_set_request
              -> cfq_get_queue
                -> cfq_find_alloc_queue
                  -> kmem_cache_alloc_node(__GFP_NOFAIL)
                    -> Jens

How much this can happen in practice I don't know, but it looks bad.

> There are other cases where __GFP_NOFAIL doesn't make sense too, and that 
> could be warned about. The __GFP_NORETRY thing was already mentioned. 
> Similarly, !__GFP_WAIT doesn't work with __GFP_NOFAIL - because the nofail 
> obviously relies on being able to do something about the failure case.
> 
> We might want to also have rules like "in order to have NOFAIL, you need 
> to allow IO and FS accesses". 

Sure, that's sane.

fs/jbd/journal.c:       new_bh = alloc_buffer_head(GFP_NOFS|__GFP_NOFAIL);

But that isn't :(

> So I don't mind warnings with __GFP_NOFAIL. I just think they should be 
> relevant, and make sense. The "order > 0" one is neither.