Re: [PATCH] mm: cma: free cma page to buddy instead of being cpuhot page

[Andrew Morton]

On Tue, 29 Oct 2013 09:33:23 +0000 Mel Gorman <mgorman@xxxxxxx> wrote:

> On Mon, Oct 28, 2013 at 07:42:49PM +0800, zhang.mingjun@xxxxxxxxxx wrote:
> > From: Mingjun Zhang <troy.zhangmingjun@xxxxxxxxxx>
> > 
> > free_contig_range frees cma pages one by one and MIGRATE_CMA pages will be
> > used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary
> > migration action when these pages reused by CMA.
> > 
> > Signed-off-by: Mingjun Zhang <troy.zhangmingjun@xxxxxxxxxx>
> > ---
> >  mm/page_alloc.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 0ee638f..84b9d84 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int cold)
> >  	 * excessively into the page allocator
> >  	 */
> >  	if (migratetype >= MIGRATE_PCPTYPES) {
> > -		if (unlikely(is_migrate_isolate(migratetype))) {
> > +		if (unlikely(is_migrate_isolate(migratetype))
> > +			|| is_migrate_cma(migratetype))
> >  			free_one_page(zone, page, 0, migratetype);
> >  			goto out;
> 
> This slightly impacts the page allocator free path for a marginal gain
> on CMA which are relatively rare allocations. There is no obvious
> benefit to this patch as I expect CMA allocations to flush the PCP lists
> when a range of pages have been isolated and migrated. Is there any
> measurable benefit to this patch?

The added overhead is pretty small - just a comparison of a local with
a constant.  And that cost is not incurred for MIGRATE_UNMOVABLE,
MIGRATE_RECLAIMABLE and MIGRATE_MOVABLE, which are the common cases
(yes?).

This thread is a bit straggly and inconclusive, but it sounds to me
that the benefit to CMA users is quite large and the cost to others is
small, so I'm inclined to run with the original patch.  Someone stop me
if that's wrong.

(we could speed up some of the migratetype tests if the MIGRATE_foo
constants were converted to bitfields.  The above test becomes "if
(migratetype & (MIGRATE_CMA|MIGRATE_ISOLATE))").

(why is is_migrate_cma() implemented as a macro in mmzone.h while
is_migrate_isolate() is an inline in page-isolation.h?)

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