[lkml]   [2009]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: upcoming item: get_page_from_freelist
On Wed, 24 Jun 2009, Andrew Morton wrote:

> On Wed, 24 Jun 2009 13:13:48 -0700 (PDT)
> 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.
> I'm unclear on precisely what you're proposing here?

It's a justification for the looping for small-order allocations.

We shouldn't actually need to free any pages to oom kill tasks since
TIF_MEMDIE allows its allocations to ignore each zone's watermarks (and it
also allows allocations to happen anywhere regardless of cpuset
restrictions just like GFP_ATOMIC), which will provide more than a "few
pages" that are already available for the oom killed task to use if each
zone's min watermark is sanely defined by min_free_kbytes, and that will
always still be above SWAP_CLUSTER_MAX for highmem.

Those memory reserves are also available for reclaimers, but PF_MEMALLOC
tasks will never loop in the page allocator because __GFP_NOFAIL is
ignored in the reclaim path (and understandably so).

> > This is not a new concept. oom has never been "immediately kill".
> Well, it has been immediate for a long time. A couple of reasons which
> I can recall:
> - A page-allocating process will oom-kill another process in the
> expectation that the killing will free up some memory. If the
> oom-killed process remains stuck in the page allocator, that doesn't
> work.

This would cause the oom killed task to repeatedly call the oom killer to
free more memory, and it would refuse to act because a task (current) is
found to have TIF_MEMDIE set in the tasklist scan. So this would be a
livelock when there is no memory left (and nothing can be reclaimed), but
that shouldn't occur with sane watermarks. The oom killed task will
immediately enter buffered_rmqueue() for each zone in the zonelist to
satisy the allocation; it would require this to return NULL every time for
it to loop endlessly. As you mentioned, this behavior has changed
recently so that TIF_MEMDIE threads loop endlessly instead of fail the
allocation attempt when get_page_from_freelist() fails with no watermarks.
The old behavior needs to be reinstated.

> - The oom-killed process might be holding locks (typically fs locks).
> This can cause an arbitrary number of other processes to be blocked.
> So to get the system unstuck we need the oom-killed process to
> immediately exit the page allocator, to handle the NULL return and to
> drop those locks.

There's no advantage to that over allowing the allocation to succeed since
current now has TIF_MEMDIE, and this is a better result than returning
NULL for ~__GFP_FS. When the old behavior of failing allocations when
ALLOC_NO_WATERMARKS has failed is restored, as you mentioned, this
livelock will be closed.

> A long time ago, the Suse kernel shipped with a largely (or
> completely?) disabled oom-killer. It removed the
> retry-small-allocations-for-ever logic and simply returned NULL to the
> caller. I never really understood what problem/thinking led Andrea to
> do that.

If you disable retries for allocations at or under
PAGE_ALLOC_COSTLY_ORDER, then the oom killer would have to be disabled
for ~__GFP_NOFAIL and ~__GFP_REPEAT as well, otherwise it would serve no
purpose for the benefit of the current context.

If the order is under PAGE_ALLOC_COSTLY_ORDER and try_to_free_pages()
returned something positive, there's a probability that we'll succeed in
the subsequent allocation attempts. If reclaim failed to free anything,
the oom killer was called (only if the order is at or under
PAGE_ALLOC_COSTLY_ORDER, the same condition we automatically loop in, and
there's again a probability that we'll succeed in subsequent allocation
attempts when that memory is freed as the result of the chosen task

In other words, the only time we wouldn't want to loop in such a case is
when the oom killer fails to kill any task, which happens when:

- another task has already been oom killed and hasn't yet exited (if
this persists, it's an indication of the problem you mentioned earlier
which should be solved by the min watermark), or

- there are no eligible tasks left to kill, in which case the machine
panics (there are only kthreads or everything else is set to

> But it's all a bit moot at present, as we seem to have removed the
> return-NULL-if-TIF_MEMDIE logic in Mel's post-2.6.30 merges. I think
> that was an accident:
> - /* This allocation should allow future memory freeing. */
> -
> rebalance:
> - if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> - && !in_interrupt()) {
> - if (!(gfp_mask & __GFP_NOMEMALLOC)) {
> -nofail_alloc:
> - /* go through the zonelist yet again, ignoring mins */
> - page = get_page_from_freelist(gfp_mask, nodemask, order,
> - zonelist, high_zoneidx, ALLOC_NO_WATERMARKS);
> - if (page)
> - goto got_pg;
> - if (gfp_mask & __GFP_NOFAIL) {
> - congestion_wait(WRITE, HZ/50);
> - goto nofail_alloc;
> - }
> - }
> - goto nopage;
> + /* Allocate without watermarks if the context allows */
> + if (alloc_flags & ALLOC_NO_WATERMARKS) {
> + page = __alloc_pages_high_priority(gfp_mask, order,
> + zonelist, high_zoneidx, nodemask,
> + preferred_zone, migratetype);
> + if (page)
> + goto got_pg;
> }
> Offending commit 341ce06 handled the PF_MEMALLOC case but forgot about
> the TIF_MEMDIE case.

Right, the allocation should fail if test_thread_flag(TIF_MEMDIE) and
__alloc_pages_high_priority() has failed. There's no need to enter
reclaim since it has already failed to free memory (the only reason it was
oom killed to begin with) and memory reserves have been fully depleted.

 \ /
  Last update: 2009-06-25 10:25    [W:0.187 / U:0.848 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site