On Sun 08-07-18 19:35:58, Tetsuo Handa wrote: > From: Michal Hocko <mhocko@xxxxxxxx> > > should_reclaim_retry() should be a natural reschedule point. PF_WQ_WORKER > is a special case which needs a stronger rescheduling policy. However, > since schedule_timeout_uninterruptible(1) for PF_WQ_WORKER depends on > __zone_watermark_ok() == true, PF_WQ_WORKER is currently counting on > mutex_trylock(&oom_lock) == 0 in __alloc_pages_may_oom() which is a bad > expectation. I think your reference to the oom_lock is more confusing than helpful actually. I would simply use the following from your previous [1] changelog: : should_reclaim_retry() should be a natural reschedule point. PF_WQ_WORKER : is a special case which needs a stronger rescheduling policy. Doing that : unconditionally seems more straightforward than depending on a zone being : a good candidate for a further reclaim. : : Thus, move the short sleep when we are waiting for the owner of oom_lock : (which coincidentally also serves as a guaranteed sleep for PF_WQ_WORKER : threads) to should_reclaim_retry(). > unconditionally seems more straightforward than depending on a zone being > a good candidate for a further reclaim. [1] http://lkml.kernel.org/r/1528369223-7571-2-git-send-email-penguin-kernel@xxxxxxxxxxxxxxxxxxx [Tetsuo: changelog] > Signed-off-by: Michal Hocko <mhocko@xxxxxxxx> > Cc: Hillf Danton <hillf.zj@xxxxxxxxxxxxxxx> > Cc: David Rientjes <rientjes@xxxxxxxxxx> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Joonsoo Kim <js1304@xxxxxxxxx> > Cc: Mel Gorman <mgorman@xxxxxxx> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> > Cc: Vladimir Davydov <vdavydov@xxxxxxxxxxxxx> > Cc: Vlastimil Babka <vbabka@xxxxxxx> Your s-o-b is still missing. > --- > mm/page_alloc.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 1521100..f56cc09 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -3922,6 +3922,7 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask) > { > struct zone *zone; > struct zoneref *z; > + bool ret = false; > > /* > * Costly allocations might have made a progress but this doesn't mean > @@ -3985,25 +3986,26 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask) > } > } > > - /* > - * Memory allocation/reclaim might be called from a WQ > - * context and the current implementation of the WQ > - * concurrency control doesn't recognize that > - * a particular WQ is congested if the worker thread is > - * looping without ever sleeping. Therefore we have to > - * do a short sleep here rather than calling > - * cond_resched(). > - */ > - if (current->flags & PF_WQ_WORKER) > - schedule_timeout_uninterruptible(1); > - else > - cond_resched(); > - > - return true; > + ret = true; > + goto out; > } > } > > - return false; > +out: > + /* > + * Memory allocation/reclaim might be called from a WQ > + * context and the current implementation of the WQ > + * concurrency control doesn't recognize that > + * a particular WQ is congested if the worker thread is > + * looping without ever sleeping. Therefore we have to > + * do a short sleep here rather than calling > + * cond_resched(). > + */ > + if (current->flags & PF_WQ_WORKER) > + schedule_timeout_uninterruptible(1); > + else > + cond_resched(); > + return ret; > } > > static inline bool > -- > 1.8.3.1 > -- Michal Hocko SUSE Labs
