Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1272016

Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without triggering reclaim

Show all headers | View raw

On Tue 17-11-15 19:58:09, Tetsuo Handa wrote:
> 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?

It does and the changelog is explicit about this.

> >   	/* 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. 

It wasn't called for PF_MEMALLOC requests though. Whether invoking OOM
killer is a good idea for this case is a harder question and out of
scope of this patch.

> 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?

As the changelog tries to clarify PF_MEMALLOC with __GFP_NOFAIL is
basically a bug. That is the reason I am adding WARN_ON there. I do not
think making this code more complex for abusers/buggy code is really
worthwhile. Besides that I fail to see why a work item would ever
want to set PF_MEMALLOC for legitimate reasons. I have done a quick git
grep over the tree and there doesn't seem to be any user.

> 
> > +		}
> >   		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);

I believe you are getting off-topic here.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/2] get rid of __alloc_pages_high_priority mhocko@kernel.org - 2015-11-16 14:30 +0100
  [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without triggering reclaim mhocko@kernel.org - 2015-11-16 14:30 +0100
    Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim David Rientjes <rientjes@google.com> - 2015-11-16 22:20 +0100
    Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2015-11-17 12:00 +0100
      Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Michal Hocko <mhocko@kernel.org> - 2015-11-18 10:20 +0100
        Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Michal Hocko <mhocko@kernel.org> - 2015-11-18 10:30 +0100
    Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Vlastimil Babka <vbabka@suse.cz> - 2015-11-18 16:00 +0100
      Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Vlastimil Babka <vbabka@suse.cz> - 2015-11-18 16:20 +0100
      Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Michal Hocko <mhocko@kernel.org> - 2015-11-18 16:20 +0100
    Re: [PATCH 2/2] mm: do not loop over ALLOC_NO_WATERMARKS without  triggering reclaim Michal Hocko <mhocko@kernel.org> - 2015-11-23 10:40 +0100
  [PATCH 1/2] mm: get rid of __alloc_pages_high_priority mhocko@kernel.org - 2015-11-16 14:30 +0100
    Re: [PATCH 1/2] mm: get rid of __alloc_pages_high_priority Mel Gorman <mgorman@suse.de> - 2015-11-16 19:50 +0100
    Re: [PATCH 1/2] mm: get rid of __alloc_pages_high_priority David Rientjes <rientjes@google.com> - 2015-11-16 22:20 +0100
    Re: [PATCH 1/2] mm: get rid of __alloc_pages_high_priority Vlastimil Babka <vbabka@suse.cz> - 2015-11-18 15:50 +0100

csiph-web