lkml.org 
[lkml]   [2015]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without triggering reclaim
From
Date
Michal Hocko wrote:
> __alloc_pages_slowpath is looping over ALLOC_NO_WATERMARKS requests if
> __GFP_NOFAIL is requested. This is fragile because we are basically
> relying on somebody else to make the reclaim (be it the direct reclaim
> or OOM killer) for us. The caller might be holding resources (e.g.
> locks) which block other other reclaimers from making any progress for
> example. Remove the retry loop and rely on __alloc_pages_slowpath to
> invoke all allowed reclaim steps and retry logic.

This implies invoking OOM killer, doesn't it?

> /* Avoid recursion of direct reclaim */
> - if (current->flags & PF_MEMALLOC)
> + if (current->flags & PF_MEMALLOC) {
> + /*
> + * __GFP_NOFAIL request from this context is rather bizarre
> + * because we cannot reclaim anything and only can loop waiting
> + * for somebody to do a work for us.
> + */
> + if (WARN_ON_ONCE(gfp_mask & __GFP_NOFAIL)) {
> + cond_resched();
> + goto retry;

I think that this "goto retry;" omits call to out_of_memory() which is allowed
for __GFP_NOFAIL allocations. Even if this is what you meant, current thread
can be a workqueue, which currently need a short sleep (as with
wait_iff_congested() changes), can't it?

> + }
> goto nopage;
> + }
>
> /* Avoid allocations with no watermarks from looping endlessly */
> if (test_thread_flag(TIF_MEMDIE) && !(gfp_mask & __GFP_NOFAIL))
>

Well, is it cond_resched() which should include

if (current->flags & PF_WQ_WORKER)
schedule_timeout(1);

than wait_iff_congested() because not all yield calls use wait_iff_congested()
and giving pending workqueue jobs a chance to be processed is anyway preferable?

int __sched _cond_resched(void)
{
if (should_resched(0)) {
if ((current->flags & PF_WQ_WORKER) && workqueue_has_pending_jobs())
schedule_timeout(1);
else
preempt_schedule_common();
return 1;
}
return 0;
}



\
 
 \ /
  Last update: 2015-11-17 12:21    [W:0.143 / U:1.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site