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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>