[PATCH] page allocator: fix update NR_FREE_PAGES only as necessary

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

> Hi, Huang. 
> 
> On Mon,  4 Jan 2010 10:22:10 +0800
> Huang Shijie <shijie8@gmail.com> wrote:
> 
> > When the `page' returned by __rmqueue() is NULL, the origin code
> > still adds -(1 << order) to zone's NR_FREE_PAGES item.
> > 
> > The patch fixes it.
> > 
> > Signed-off-by: Huang Shijie <shijie8@gmail.com>
> > ---
> >  mm/page_alloc.c |   10 +++++++---
> >  1 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 4e9f5cc..620921d 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1222,10 +1222,14 @@ again:
> >  		}
> >  		spin_lock_irqsave(&zone->lock, flags);
> >  		page = __rmqueue(zone, order, migratetype);
> > -		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
> > -		spin_unlock(&zone->lock);
> > -		if (!page)
> > +		if (likely(page)) {
> > +			__mod_zone_page_state(zone, NR_FREE_PAGES,
> > +						-(1 << order));
> > +			spin_unlock(&zone->lock);
> > +		} else {
> > +			spin_unlock(&zone->lock);
> >  			goto failed;
> > +		}
> >  	}
> >  
> >  	__count_zone_vm_events(PGALLOC, zone, 1 << order);
> 
> I think it's not desirable to add new branch in hot-path even though
> we could avoid that. 
> 
> How about this?
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4e4b5b3..87976ad 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1244,6 +1244,9 @@ again:
>         return page;
>  
>  failed:
> +       spin_lock(&zone->lock);
> +       __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
> +       spin_unlock(&zone->lock);
>         local_irq_restore(flags);
>         put_cpu();
>         return NULL;

Why can't we write following? __mod_zone_page_state() only require irq
disabling, it doesn't need spin lock. I think.



From 72011ff2b0bba6544ae35c6ee52715c8c824a34b Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Mon, 4 Jan 2010 14:38:20 +0900
Subject: [PATCH] page allocator: fix update NR_FREE_PAGES only as necessary

commit f2260e6b (page allocator: update NR_FREE_PAGES only as necessary)
made one minor regression.
if __rmqueue() was failed, NR_FREE_PAGES stat go wrong. this patch fixes
it.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Huang Shijie <shijie8@gmail.com>
Cc: Minchan Kim <minchan.kim@gmail.com>
---
 mm/page_alloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 11ae66e..ecf75a1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1227,10 +1227,10 @@ again:
 		}
 		spin_lock_irqsave(&zone->lock, flags);
 		page = __rmqueue(zone, order, migratetype);
-		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
 		spin_unlock(&zone->lock);
 		if (!page)
 			goto failed;
+		__mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
 	}
 
 	__count_zone_vm_events(PGALLOC, zone, 1 << order);
-- 
1.6.5.2




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