Re: [patch v3 6/6] mm, compaction: terminate async compaction when rescheduling

David Rientjes
2014-05-07T21:28:42Z

On Wed, 7 May 2014, Andrew Morton wrote:

> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -500,8 +500,13 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
> > return 0;
> > }
> >
> > + if (cond_resched()) {
> > + /* Async terminates prematurely on need_resched() */
> > + if (cc->mode == MIGRATE_ASYNC)
> > + return 0;
> > + }
>
> Comment comments the obvious. What is less obvious is *why* we do this.
>

Async compaction is most prevalent for thp pagefaults and without
zone->lru_lock contention we have no other termination criteria. Without
this, we would scan a potentially very long zone (zones 64GB in length in
my testing) and it would be very expensive for pagefault. Async is best
effort, so if it is becoming too expensive then it's better to just
fallback to PAGE_SIZE pages instead and rely on khugepaged to collapse
later.

> Someone please remind my why sync and async compaction use different
> scanning cursors?
>

It's introduced in this patchset. Async compaction does not consider
pageblocks unless it is MIGRATE_MOVABLE since it is best effort, sync
compaction considers all pageblocks. In the past, we only updated the
cursor for sync compaction since it would be wrong to update it for async
compaction if it can skip certain pageblocks. Unfortunately, if async
compaction is relied upon solely for certain allocations (such as thp
pagefaults), it is possible to scan an enormous amount of a 64GB zone, for
example, pointlessly every time if none of the memory can be isolated.

The result is that sync compaction always updates both scanners and async
compaction only updates its own scanner. Either scanner is only updated
if the new cursor is "beyond" the previous cursor. ("Beyond" is _after_
the previous migration scanner pfn and _before_ the previous free scanner
pfn.)