Re: [PATCH] cma: cached pageblock type fixup

[Mel Gorman <mgorman@suse.de>]

On Wed, May 23, 2012 at 09:22:00AM +0200, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 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?

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.

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.

-- 
Mel Gorman
SUSE Labs

--
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>