On Tue, May 31, 2016 at 02:29:24PM +0200, Vlastimil Babka wrote: > On 05/31/2016 02:07 PM, Vlastimil Babka wrote: > >On 05/31/2016 08:37 AM, Joonsoo Kim wrote: > >>>@@ -3695,22 +3695,22 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned > >>>int order, > >>> else > >>> no_progress_loops++; > >>> > >>>- if (should_reclaim_retry(gfp_mask, order, ac, alloc_flags, > >>>- did_some_progress > 0, no_progress_loops)) > >>>- goto retry; > >>>- > >>>+ should_retry = should_reclaim_retry(gfp_mask, order, ac, alloc_flags, > >>>+ did_some_progress > 0, no_progress_loops); > >>> /* > >>> * It doesn't make any sense to retry for the compaction if the order-0 > >>> * reclaim is not able to make any progress because the current > >>> * implementation of the compaction depends on the sufficient amount > >>> * of free memory (see __compaction_suitable) > >>> */ > >>>- if (did_some_progress > 0 && > >>>- should_compact_retry(ac, order, alloc_flags, > >>>+ if (did_some_progress > 0) > >>>+ should_retry |= should_compact_retry(ac, order, alloc_flags, > >>> compact_result, &compact_priority, > >>>- compaction_retries)) > >>>+ compaction_retries); > >>>+ if (should_retry) > >>> goto retry; > >> > >>Hmm... it looks odd that we check should_compact_retry() when > >>did_some_progress > 0. If system is full of anonymous memory and we > >>don't have swap, we can't reclaim anything but we can compact. > > > >Right, thanks. > > Hmm on the other hand, should_compact_retry will assume (in > compaction_zonelist_suitable()) that reclaimable memory is actually > reclaimable. If there's nothing to tell us that it actually isn't, > if we drop the reclaim progress requirement. That's risking an > infinite loop?
You are right. I hope this retry logic will be robust to cover all the theoretical situations but it looks not easy. Sigh... Thanks.
