lkml.org 
[lkml]   [2009]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: upcoming kerneloops.org item: get_page_from_freelist
On Tue, Jun 30, 2009 at 01:13:08AM -0700, David Rientjes wrote:
> On Tue, 30 Jun 2009, Nick Piggin wrote:
>
> > > That's not the expected behavior for TIF_MEMDIE, although your patch
> > > certainly changes that.
> > >
> > > Your patch is simply doing
> > >
> > > if (test_thread_flag(TIF_MEMDIE))
> > > gfp_mask |= __GFP_NORETRY;
> > >
> > > in the slowpath.
> > >
> > > TIF_MEMDIE is supposed to allow allocations to succeed, not automatically
> > > fail, so that it can quickly handle its SIGKILL without getting blocked in
> > > the exit path seeking more memory.
> >
> > Yes, it need to just ignore all watermarks, do not reclaim (we've
> > already decided reclaim will not work at this point), and return a
> > page if we have one otherwise NULL (unless GFP_NOFAIL is set).
> >
>
> Right, there's no sense in looping endlessly for ~__GFP_NOFAIL if
> allocations continue to fail for a thread with TIF_MEMDIE set.
>
> TIF_MEMDIE doesn't check any watermarks as opposed to GFP_ATOMIC, which
> only reduces the min watermark by half, so we can access more memory
> reserves with TIF_MEMDIE. Instead of immediately failing an oom killed
> task's allocation as in Mel's patch, there is a higher liklihood that it
> will succeed on the next attempt.

Yes. This is how it should have worked prior to Mel's patches, so we
should aim to restore that.


> I'd agree with Mel's added check for TIF_MEMDIE upon returning from the
> oom killer, but only for __GFP_NOMEMALLOC.

NOMEMALLOC indeed should always be kept away from memalloc/memdie
reserves. That's how it should have worked when I added it (but
I may have forgotten TIF_MEMDIE, I can't remember).


> > > All __GFP_NOFAIL allocations should ensure that alloc_pages() never
> > > returns NULL. Although it's unfortunate, that's the requirement that
> > > callers have been guaranteed and until they are fixed, the page allocator
> > > should respect it.
> >
> > Yes.
> >
> > Interesting thing is what to do when we have 0 pages left, we are
> > TIF_MEMDIE, and GFP_NOFAIL is set. Looping will most likely just
> > deadlock the system. Returning NULL will probably oops caller with
> > various locks held and then deadlock the system. It really needs to
> > punt back to the OOM killer so it can select another task. Until
> > then, maybe a simple panic would be reasonable? (it's *never* going
> > to hit anyone in practice I'd say, but if it does then a panic
> > would be better than lockup at least we know what the problem was).
> >
>
> The oom killer currently is a no-op if any eligible task has TIF_MEMDIE,
> so this would require adding an oom killer timeout so that if a task fails
> to exit after a predefined period, TIF_MEMDIE is cleared and the task is
> marked to no longer be selected (which would require an addition to
> task_struct) although it may have already completely depleted memory
> reserves.

It wouldn't have to be a timeout, it could be a call back to the
oom killer.


> The best alternative is just to increase min_free_kbytes to ensure that
> adequate memory reserves (and its partial exemptions allowed by
> GFP_ATOMIC, ALLOC_HARDER, and PF_MEMALLOC) are sustained for an oom killed
> task to exit and that we try hard to avoid getting stuck in
> TASK_UNINTERRUPTIBLE.

Well we're discussing what to do when reserves run out and NOFAIL
is set. So increasing min_free_kbytes is not a valid alternative :)
My vote is a simple panic with a clear message.


> > > I disagree with this change because it unconditionally fails allocations
> > > when a task has been oom killed, a scenario which should be the _highest_
> > > priority for allocations to succeed since it leads to future memory
> >
> > That's another interesting point. I do agree with you because that
> > would restore previous behaviour which got broken. But I wonder if
> > possibly it would be a better idea to fail all allocations? That
> > would a) protect reserves more, and b) probably quite likely to
> > exit the syscall *sooner* than if we try to satisfy all allocations.
> >
>
> You could only fail the single allocation where you triggered the oom
> killer and you were the task chosen to die, which is what Mel's patch
> implemented in the first half. I agree that would protect the memory
> reserves more.



\
 
 \ /
  Last update: 2009-06-30 10:27    [W:1.058 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site