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?

Reply via email to