Re: [patch v2 3/4] mm, compaction: add per-zone migration pfn cache for async compaction

[Vlastimil Babka]

On 05/06/2014 02:29 AM, David Rientjes wrote:
> On Mon, 5 May 2014, Vlastimil Babka wrote:
>
> > I see, although I would still welcome some numbers to back such change.
>
> It's pretty difficult to capture numbers for this in real-world scenarios
> since it happens rarely (and when it happens, it's very significant
> latency) and without an instrumented kernel that will determine how many
> pageblocks have been skipped.  I could create a synthetic example of it in
> the kernel and get numbers for a worst-case scenario with a 64GB zone if
> you'd like, I'm not sure how representative it will be.
>
> > What I still don't like is the removal of the intent of commit 50b5b094e6. You
> > now again call set_pageblock_skip() unconditionally, thus also on pageblocks
> > that async compaction skipped due to being non-MOVABLE. The sync compaction
> > will thus ignore them.
>
> I'm not following you, with this patch there are two cached pfns for the
> migration scanner: one is used for sync and one is used for async.  When
> cc->sync == true, both cached pfns are updated (async is not going to
> succeed for a pageblock when sync failed for that pageblock); when
> cc->sync == false, the async cached pfn is updated only and we pick up
> again where we left off for subsequent async compactions.  Sync compaction
> will still begin where it last left off and consider these non-MOVABLE
> pageblocks.

Yeah I understand, the cached pfn's are not the problem. The problem is 
that with your patch, set_pageblock_skip() will be called through 
update_pageblock_skip() in async compaction, since you removed the 
skipped_async_unsuitable variable. So in sync compaction, such pageblock 
will be skipped thanks to the isolation_suitable() check which uses 
get_pageblock_skip() to read the bit set by the async compaction.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/