Michal Hocko wrote:
> On Sat 12-05-18 23:18:24, Tetsuo Handa wrote:
> [...]
> > @@ -4241,6 +4240,12 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
> > /* Retry as long as the OOM killer is making progress */
> > if (did_some_progress) {
> > no_progress_loops = 0;
> > + /*
> > + * This schedule_timeout_*() serves as a guaranteed sleep for
> > + * PF_WQ_WORKER threads when __zone_watermark_ok() == false.
> > + */
> > + if (!tsk_is_oom_victim(current))
> > + schedule_timeout_uninterruptible(1);
> > goto retry;
>
> We already do have that sleep for PF_WQ_WORKER in should_reclaim_retry.
> Why do we need it here as well?
Because that path depends on __zone_watermark_ok() == true which is not
guaranteed to be executed.
I consider that this "goto retry;" is a good location for making a short sleep.
Current code is so conditional that there are cases which needlessly retry
without sleeping (e.g. current thread finds an OOM victim at select_bad_process()
and immediately retries allocation attempt rather than giving the OOM victim
CPU resource for releasing memory) or needlessly sleep (e.g. current thread
was selected as an OOM victim but mutex_trylock(&oom_lock) in
__alloc_pages_may_oom() failed).
