Thread (7 messages) 7 messages, 3 authors, 2012-06-19

Re: [PATCH] cma: cached pageblock type fixup

From: Bartlomiej Zolnierkiewicz <hidden>
Date: 2012-06-19 11:29:33

On Tuesday 19 June 2012 11:00:15 Mel Gorman wrote:
On Wed, May 23, 2012 at 09:22:00AM +0200, Bartlomiej Zolnierkiewicz wrote:
quoted
From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH] cma: cached pageblock type fixup

CMA pages added to per-cpu pages lists in free_hot_cold_page()
have private field set to MIGRATE_CMA pageblock type .  If this
happes just before start_isolate_page_range() in alloc_contig_range()
changes pageblock type of the page to MIGRATE_ISOLATE it may result
in the cached pageblock type being stale in free_pcppages_bulk()
(which may be triggered by drain_all_pages() in alloc_contig_range()),
So what?
"page being added to MIGRATE_CMA free list instead of MIGRATE_ISOLATE
one in __free_one_page() and (if the page is reused just before
test_pages_isolated() check) causing alloc_contig_range() failure."
The pages get freed to the MIGRATE_CMA region. At worst they will be
used for an allocation request that is compatible with being migrated by
CMA. This will delay the allocation time of alloc_contig_range() but is
hardly critical.
There is no waiting on MIGRATE_CMA pages in alloc_contig_range() to
become free again.  If the conversion to MIGRATE_ISOLATE fails then
the whole alloc_contig_range() allocation fails on test_pages_isolated()
check.
Your fix on the other hand adds another call to get_pageblock_type() to
free_pcppages_bulk which is expensive. The change made to
buffered_rmqueue() is horrific. It takes the per-cpu page allocation
path and adds a spin lock to it which completely defeats the purpose of
having the per-cpu allocation avoid taking locks. This will have a very
heavy impact on performance, particularly on parallel workloads.

As the impact of the race should be marginal and the cost of the fix is
so unbelivably high I'm nacking this patch. If this race is a problem then
it should be handled in alloc_contig_range() not in the allocator fast paths.
Do you have any idea how this race can be handled in alloc_contig_range()?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help