On Wed 04-05-16 15:31:12, Joonsoo Kim wrote: > On Wed, May 04, 2016 at 03:01:24PM +0900, Joonsoo Kim wrote: > > On Wed, Apr 20, 2016 at 03:47:25PM -0400, Michal Hocko wrote: [...] > > > @@ -3408,6 +3456,17 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned > > > int order, > > > no_progress_loops)) > > > goto retry; > > > > > > + /* > > > + * 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(order, compact_result, > > > + &migration_mode, compaction_retries)) > > > > Checking did_some_progress on each round have subtle corner case. Think > > about following situation. > > > > round, compaction, did_some_progress, compaction > > 0, defer, 1 > > 0, defer, 1 > > 0, defer, 1 > > 0, defer, 1 > > 0, defer, 0 > > Oops...Example should be below one. > > 0, defer, 1 > 1, defer, 1 > 2, defer, 1 > 3, defer, 1 > 4, defer, 0
I am not sure I understand. The point of the check is that if the reclaim doesn't make _any_ progress then checking the result of the compaction after it didn't lead to a successful allocation just doesn't make any sense. If the compaction deferred all the time then we have a bug in the compaction. Vlastimil is already working on a code which should make the compaction more ready for !costly requests but that is a separate topic IMO. -- Michal Hocko SUSE Labs
