linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: kosaki.motohiro@jp.fujitsu.com,
	Wu Fengguang <fengguang.wu@intel.com>,
	Minchan Kim <minchan.kim@gmail.com>,
	Huang Shijie <shijie8@gmail.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"mel@csn.ul.ie" <mel@csn.ul.ie>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Rik van Riel <riel@redhat.com>
Subject: Re: [PATCH 4/4] mm/page_alloc : relieve zone->lock's pressure for memory free
Date: Tue, 12 Jan 2010 14:10:36 +0900 (JST)	[thread overview]
Message-ID: <20100112140923.B3A4.A69D9226@jp.fujitsu.com> (raw)
In-Reply-To: <20100112133223.005b81ed.kamezawa.hiroyu@jp.fujitsu.com>

> On Tue, 12 Jan 2010 12:21:16 +0800
> Wu Fengguang <fengguang.wu@intel.com> wrote:
> > > BTW,
> > > Hmm. It's not atomic as Kame pointed out.
> > > 
> > > Now, zone->flags have several bit.
> > >  * ZONE_ALL_UNRECLAIMALBE
> > >  * ZONE_RECLAIM_LOCKED
> > >  * ZONE_OOM_LOCKED.
> > > 
> > > I think this flags are likely to race when the memory pressure is high.
> > > If we don't prevent race, concurrent reclaim and killing could be happened.
> > > So I think reset zone->flags outside of zone->lock would make our efforts which
> > > prevent current reclaim and killing invalidate.
> > 
> > zone_set_flag()/zone_clear_flag() calls set_bit()/clear_bit() which is
> > atomic. Do you mean more high level exclusion?
> > 
> Ah, sorry, I missed that.
> In my memory, this wasn't atomic ;) ...maybe recent change.
> 
> I don't want to see atomic_ops here...So, how about making this back to be
> zone->all_unreclaimable word ?
> 
> Clearing this is not necessary to be atomic because this is cleard at every
> page freeing.

I agree. How about this?



From 751f197ad256c7245151681d7aece591b1dab343 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Tue, 12 Jan 2010 13:53:47 +0900
Subject: [PATCH] mm: Restore zone->all_unreclaimable to independence word

commit e815af95 (change all_unreclaimable zone member to flags) chage
all_unreclaimable member to bit flag. but It have undesireble side
effect.
free_one_page() is one of most hot path in linux kernel and increasing
atomic ops in it can reduce kernel performance a bit.

Thus, this patch revert such commit partially. at least
all_unreclaimable shouldn't share memory word with other zone flags.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 include/linux/mmzone.h |    7 +------
 mm/page_alloc.c        |    6 +++---
 mm/vmscan.c            |   20 ++++++++------------
 mm/vmstat.c            |    2 +-
 4 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 30fe668..4f0c6f1 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -341,6 +341,7 @@ struct zone {
 
 	unsigned long		pages_scanned;	   /* since last reclaim */
 	unsigned long		flags;		   /* zone flags, see below */
+	int                     all_unreclaimable; /* All pages pinned */
 
 	/* Zone statistics */
 	atomic_long_t		vm_stat[NR_VM_ZONE_STAT_ITEMS];
@@ -425,7 +426,6 @@ struct zone {
 } ____cacheline_internodealigned_in_smp;
 
 typedef enum {
-	ZONE_ALL_UNRECLAIMABLE,		/* all pages pinned */
 	ZONE_RECLAIM_LOCKED,		/* prevents concurrent reclaim */
 	ZONE_OOM_LOCKED,		/* zone is in OOM killer zonelist */
 } zone_flags_t;
@@ -445,11 +445,6 @@ static inline void zone_clear_flag(struct zone *zone, zone_flags_t flag)
 	clear_bit(flag, &zone->flags);
 }
 
-static inline int zone_is_all_unreclaimable(const struct zone *zone)
-{
-	return test_bit(ZONE_ALL_UNRECLAIMABLE, &zone->flags);
-}
-
 static inline int zone_is_reclaim_locked(const struct zone *zone)
 {
 	return test_bit(ZONE_RECLAIM_LOCKED, &zone->flags);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4e9f5cc..19a5b0e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -530,7 +530,7 @@ static void free_pcppages_bulk(struct zone *zone, int count,
 	int batch_free = 0;
 
 	spin_lock(&zone->lock);
-	zone_clear_flag(zone, ZONE_ALL_UNRECLAIMABLE);
+	zone->all_unreclaimable = 0;
 	zone->pages_scanned = 0;
 
 	__mod_zone_page_state(zone, NR_FREE_PAGES, count);
@@ -567,7 +567,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
 				int migratetype)
 {
 	spin_lock(&zone->lock);
-	zone_clear_flag(zone, ZONE_ALL_UNRECLAIMABLE);
+	zone->all_unreclaimable = 0;
 	zone->pages_scanned = 0;
 
 	__mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
@@ -2270,7 +2270,7 @@ void show_free_areas(void)
 			K(zone_page_state(zone, NR_BOUNCE)),
 			K(zone_page_state(zone, NR_WRITEBACK_TEMP)),
 			zone->pages_scanned,
-			(zone_is_all_unreclaimable(zone) ? "yes" : "no")
+			(zone->all_unreclaimable ? "yes" : "no")
 			);
 		printk("lowmem_reserve[]:");
 		for (i = 0; i < MAX_NR_ZONES; i++)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 885207a..8057d36 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1694,8 +1694,7 @@ static void shrink_zones(int priority, struct zonelist *zonelist,
 				continue;
 			note_zone_scanning_priority(zone, priority);
 
-			if (zone_is_all_unreclaimable(zone) &&
-						priority != DEF_PRIORITY)
+			if (zone->all_unreclaimable && priority != DEF_PRIORITY)
 				continue;	/* Let kswapd poll it */
 			sc->all_unreclaimable = 0;
 		} else {
@@ -2009,8 +2008,7 @@ loop_again:
 			if (!populated_zone(zone))
 				continue;
 
-			if (zone_is_all_unreclaimable(zone) &&
-			    priority != DEF_PRIORITY)
+			if (zone->all_unreclaimable && priority != DEF_PRIORITY)
 				continue;
 
 			/*
@@ -2053,8 +2051,7 @@ loop_again:
 			if (!populated_zone(zone))
 				continue;
 
-			if (zone_is_all_unreclaimable(zone) &&
-					priority != DEF_PRIORITY)
+			if (zone->all_unreclaimable && priority != DEF_PRIORITY)
 				continue;
 
 			if (!zone_watermark_ok(zone, order,
@@ -2084,12 +2081,11 @@ loop_again:
 						lru_pages);
 			sc.nr_reclaimed += reclaim_state->reclaimed_slab;
 			total_scanned += sc.nr_scanned;
-			if (zone_is_all_unreclaimable(zone))
+			if (zone->all_unreclaimable)
 				continue;
-			if (nr_slab == 0 && zone->pages_scanned >=
-					(zone_reclaimable_pages(zone) * 6))
-					zone_set_flag(zone,
-						      ZONE_ALL_UNRECLAIMABLE);
+			if (nr_slab == 0 &&
+			    zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
+				zone->all_unreclaimable = 1;
 			/*
 			 * If we've done a decent amount of scanning and
 			 * the reclaim ratio is low, start doing writepage
@@ -2612,7 +2608,7 @@ int zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
 	    zone_page_state(zone, NR_SLAB_RECLAIMABLE) <= zone->min_slab_pages)
 		return ZONE_RECLAIM_FULL;
 
-	if (zone_is_all_unreclaimable(zone))
+	if (zone->all_unreclaimable)
 		return ZONE_RECLAIM_FULL;
 
 	/*
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 6051fba..8175c64 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -761,7 +761,7 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
 		   "\n  prev_priority:     %i"
 		   "\n  start_pfn:         %lu"
 		   "\n  inactive_ratio:    %u",
-			   zone_is_all_unreclaimable(zone),
+		   zone->all_unreclaimable,
 		   zone->prev_priority,
 		   zone->zone_start_pfn,
 		   zone->inactive_ratio);
-- 
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>

  parent reply	other threads:[~2010-01-12  5:10 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-11  4:37 [PATCH 1/4] mm/page_alloc : rename rmqueue_bulk to rmqueue_single Huang Shijie
2010-01-11  4:37 ` [PATCH 2/4] mm/page_alloc : relieve the zone->lock's pressure for allocation Huang Shijie
2010-01-11  4:37   ` [PATCH 3/4] mm/page_alloc : modify the return type of __free_one_page Huang Shijie
2010-01-11  4:37     ` [PATCH 4/4] mm/page_alloc : relieve zone->lock's pressure for memory free Huang Shijie
2010-01-11  5:20       ` Minchan Kim
2010-01-11  6:01         ` Huang Shijie
2010-01-11  6:27       ` Huang Shijie
2010-01-11  6:38         ` Minchan Kim
2010-01-11  6:59           ` Huang Shijie
2010-01-12  0:47           ` KAMEZAWA Hiroyuki
2010-01-12  2:02             ` Huang Shijie
2010-01-12  2:07               ` KAMEZAWA Hiroyuki
2010-01-12  2:32                 ` Huang Shijie
2010-01-12  2:27             ` Wu Fengguang
2010-01-12  2:56               ` KOSAKI Motohiro
2010-01-12  3:02                 ` Huang Shijie
2010-01-12  4:05               ` Minchan Kim
2010-01-12  4:21                 ` Wu Fengguang
2010-01-12  4:32                   ` KAMEZAWA Hiroyuki
2010-01-12  4:59                     ` Wu Fengguang
2010-01-12  5:09                       ` Wu Fengguang
2010-01-12  5:10                     ` KOSAKI Motohiro [this message]
2010-01-12  7:36                       ` David Rientjes
2010-01-12  8:56                         ` KOSAKI Motohiro
2010-01-12 21:39                           ` David Rientjes
2010-01-13  0:01                             ` KOSAKI Motohiro
2010-01-12  4:48                   ` Minchan Kim
2010-01-12  2:51       ` Huang Shijie
2010-01-12  3:03         ` Wu Fengguang
2010-01-12  3:05         ` KOSAKI Motohiro
2010-01-11  5:04     ` [PATCH 3/4] mm/page_alloc : modify the return type of __free_one_page Minchan Kim
2010-01-12  2:56     ` KOSAKI Motohiro
2010-01-18 11:25     ` Mel Gorman
2010-01-19  1:49       ` Huang Shijie
2010-01-11  5:02   ` [PATCH 2/4] mm/page_alloc : relieve the zone->lock's pressure for allocation Minchan Kim
2010-01-11  5:13     ` Huang Shijie
2010-01-12  2:54   ` KOSAKI Motohiro
2010-01-18 11:24   ` Mel Gorman
2010-01-11  5:00 ` [PATCH 1/4] mm/page_alloc : rename rmqueue_bulk to rmqueue_single Minchan Kim
2010-01-12  2:52 ` KOSAKI Motohiro
2010-01-18 11:21 ` Mel Gorman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100112140923.B3A4.A69D9226@jp.fujitsu.com \
    --to=kosaki.motohiro@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=fengguang.wu@intel.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=minchan.kim@gmail.com \
    --cc=riel@redhat.com \
    --cc=shijie8@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox