linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
To: Konstantin Khlebnikov <khlebnikov@openvz.org>
Cc: Hugh Dickins <hughd@google.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH v3 20/21] mm: split zone->lru_lock
Date: Tue, 28 Feb 2012 10:49:13 +0900	[thread overview]
Message-ID: <20120228104913.0fc13bfd.kamezawa.hiroyu@jp.fujitsu.com> (raw)
In-Reply-To: <20120223135323.12988.1605.stgit@zurg>

On Thu, 23 Feb 2012 17:53:23 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:

> Looks like all ready for splitting zone->lru_lock into per-lruvec pieces.
> 
> lruvec locking loop protected with rcu, actually there is irq-disabling instead
> of rcu_read_lock(). Memory controller already releases its lru-vectors after
> syncronize_rcu() in cgroup_diput(). Probably it should be replaced with synchronize_sched()
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>

Do we need rcu_read_lock() even if we check isolated pages at pre_destroy() ?
If pre_destroy() ends, pages under a memcg being destroyed were moved to other
cgroup while it's usolated.

So, 
 - PageLRU(page) guarantees lruvec is valid.
 - if !PageLRU(page), the caller of lru_lock should know what it does.
   Once isolated, pre_destroy() never ends and page_lruvec(page) is always stable.

Thanks,
-Kame

> ---
>  include/linux/mmzone.h |    3 +-
>  mm/compaction.c        |    2 +
>  mm/internal.h          |   66 +++++++++++++++++++++++++-----------------------
>  mm/page_alloc.c        |    2 +
>  mm/swap.c              |    2 +
>  5 files changed, 40 insertions(+), 35 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 2e3a298..9880150 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -304,6 +304,8 @@ struct zone_reclaim_stat {
>  };
>  
>  struct lruvec {
> +	spinlock_t		lru_lock;
> +
>  	struct list_head	pages_lru[NR_LRU_LISTS];
>  	unsigned long		pages_count[NR_LRU_COUNTERS];
>  
> @@ -386,7 +388,6 @@ struct zone {
>  	ZONE_PADDING(_pad1_)
>  
>  	/* Fields commonly accessed by the page reclaim scanner */
> -	spinlock_t		lru_lock;
>  	struct lruvec		lruvec;
>  
>  	unsigned long		pages_scanned;	   /* since last reclaim */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index fa74cbe..8661bb58 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -306,7 +306,7 @@ static isolate_migrate_t isolate_migratepages(struct zone *zone,
>  			lruvec = NULL;
>  		}
>  		if (need_resched() ||
> -		    (lruvec && spin_is_contended(&zone->lru_lock))) {
> +		    (lruvec && spin_is_contended(&lruvec->lru_lock))) {
>  			if (lruvec)
>  				unlock_lruvec_irq(lruvec);
>  			lruvec = NULL;
> diff --git a/mm/internal.h b/mm/internal.h
> index 6dd2e70..9a9fd53 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -15,27 +15,27 @@
>  
>  static inline void lock_lruvec(struct lruvec *lruvec, unsigned long *flags)
>  {
> -	spin_lock_irqsave(&lruvec_zone(lruvec)->lru_lock, *flags);
> +	spin_lock_irqsave(&lruvec->lru_lock, *flags);
>  }
>  
>  static inline void lock_lruvec_irq(struct lruvec *lruvec)
>  {
> -	spin_lock_irq(&lruvec_zone(lruvec)->lru_lock);
> +	spin_lock_irq(&lruvec->lru_lock);
>  }
>  
>  static inline void unlock_lruvec(struct lruvec *lruvec, unsigned long *flags)
>  {
> -	spin_unlock_irqrestore(&lruvec_zone(lruvec)->lru_lock, *flags);
> +	spin_unlock_irqrestore(&lruvec->lru_lock, *flags);
>  }
>  
>  static inline void unlock_lruvec_irq(struct lruvec *lruvec)
>  {
> -	spin_unlock_irq(&lruvec_zone(lruvec)->lru_lock);
> +	spin_unlock_irq(&lruvec->lru_lock);
>  }
>  
>  static inline void wait_lruvec_unlock(struct lruvec *lruvec)
>  {
> -	spin_unlock_wait(&lruvec_zone(lruvec)->lru_lock);
> +	spin_unlock_wait(&lruvec->lru_lock);
>  }
>  
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
> @@ -46,37 +46,39 @@ static inline void wait_lruvec_unlock(struct lruvec *lruvec)
>  static inline struct lruvec *__relock_page_lruvec(struct lruvec *locked_lruvec,
>  						  struct page *page)
>  {
> -	/* Currenyly only one lru_lock per-zone */
> -	return page_lruvec(page);
> +	struct lruvec *lruvec;
> +
> +	do {
> +		lruvec = page_lruvec(page);
> +		if (likely(lruvec == locked_lruvec))
> +			return lruvec;
> +		spin_unlock(&locked_lruvec->lru_lock);
> +		spin_lock(&lruvec->lru_lock);
> +		locked_lruvec = lruvec;
> +	} while (1);
>  }
>  
>  static inline struct lruvec *relock_page_lruvec_irq(struct lruvec *lruvec,
>  						    struct page *page)
>  {
> -	struct zone *zone = page_zone(page);
> -
>  	if (!lruvec) {
> -		spin_lock_irq(&zone->lru_lock);
> -	} else if (zone != lruvec_zone(lruvec)) {
> -		unlock_lruvec_irq(lruvec);
> -		spin_lock_irq(&zone->lru_lock);
> +		local_irq_disable();
> +		lruvec = page_lruvec(page);
> +		spin_lock(&lruvec->lru_lock);
>  	}
> -	return page_lruvec(page);
> +	return __relock_page_lruvec(lruvec, page);
>  }
>  
>  static inline struct lruvec *relock_page_lruvec(struct lruvec *lruvec,
>  						struct page *page,
>  						unsigned long *flags)
>  {
> -	struct zone *zone = page_zone(page);
> -
>  	if (!lruvec) {
> -		spin_lock_irqsave(&zone->lru_lock, *flags);
> -	} else if (zone != lruvec_zone(lruvec)) {
> -		unlock_lruvec(lruvec, flags);
> -		spin_lock_irqsave(&zone->lru_lock, *flags);
> +		local_irq_save(*flags);
> +		lruvec = page_lruvec(page);
> +		spin_lock(&lruvec->lru_lock);
>  	}
> -	return page_lruvec(page);
> +	return __relock_page_lruvec(lruvec, page);
>  }
>  
>  /*
> @@ -87,22 +89,24 @@ static inline struct lruvec *relock_page_lruvec(struct lruvec *lruvec,
>  static inline bool __lock_page_lruvec_irq(struct lruvec **lruvec,
>  					  struct page *page)
>  {
> -	struct zone *zone;
>  	bool ret = false;
>  
> +	rcu_read_lock();
> +	/*
> +	 * If we see there PageLRU(), it means page has valid lruvec link.
> +	 * We need protect whole operation with single rcu-interval, otherwise
> +	 * lruvec which hold this LRU sign can run out before we secure it.
> +	 */
>  	if (PageLRU(page)) {
>  		if (!*lruvec) {
> -			zone = page_zone(page);
> -			spin_lock_irq(&zone->lru_lock);
> -		} else
> -			zone = lruvec_zone(*lruvec);
> -
> -		if (PageLRU(page)) {
>  			*lruvec = page_lruvec(page);
> +			lock_lruvec_irq(*lruvec);
> +		}
> +		*lruvec = __relock_page_lruvec(*lruvec, page);
> +		if (PageLRU(page))
>  			ret = true;
> -		} else
> -			*lruvec = &zone->lruvec;
>  	}
> +	rcu_read_unlock();
>  
>  	return ret;
>  }
> @@ -110,7 +114,7 @@ static inline bool __lock_page_lruvec_irq(struct lruvec **lruvec,
>  /* Wait for lruvec unlock before locking other lruvec for the same page */
>  static inline void __wait_lruvec_unlock(struct lruvec *lruvec)
>  {
> -	/* Currently only one lru_lock per-zone */
> +	wait_lruvec_unlock(lruvec);
>  }
>  
>  #else /* CONFIG_CGROUP_MEM_RES_CTLR */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index ab42446..beadcc9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4294,6 +4294,7 @@ void init_zone_lruvec(struct zone *zone, struct lruvec *lruvec)
>  	enum lru_list lru;
>  
>  	memset(lruvec, 0, sizeof(struct lruvec));
> +	spin_lock_init(&lruvec->lru_lock);
>  	for_each_lru(lru)
>  		INIT_LIST_HEAD(&lruvec->pages_lru[lru]);
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR
> @@ -4369,7 +4370,6 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat,
>  #endif
>  		zone->name = zone_names[j];
>  		spin_lock_init(&zone->lock);
> -		spin_lock_init(&zone->lru_lock);
>  		zone_seqlock_init(zone);
>  		zone->zone_pgdat = pgdat;
>  
> diff --git a/mm/swap.c b/mm/swap.c
> index 998c71c..8156181 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -700,7 +700,7 @@ void lru_add_page_tail(struct lruvec *lruvec,
>  	VM_BUG_ON(!PageHead(page));
>  	VM_BUG_ON(PageCompound(page_tail));
>  	VM_BUG_ON(PageLRU(page_tail));
> -	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&lruvec_zone(lruvec)->lru_lock));
> +	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&lruvec->lru_lock));
>  
>  	SetPageLRU(page_tail);
>  
> 
> 

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-02-28  1:50 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23 13:51 [PATCH v3 00/21] mm: lru_lock splitting Konstantin Khlebnikov
2012-02-23 13:51 ` [PATCH v3 01/21] memcg: unify inactive_ratio calculation Konstantin Khlebnikov
2012-02-28  0:05   ` KAMEZAWA Hiroyuki
2012-02-23 13:51 ` [PATCH v3 02/21] memcg: make mm_match_cgroup() hirarchical Konstantin Khlebnikov
2012-02-23 18:03   ` Johannes Weiner
2012-02-23 19:46     ` Konstantin Khlebnikov
2012-02-23 22:06       ` Johannes Weiner
2012-02-28  0:11   ` KAMEZAWA Hiroyuki
2012-02-28  6:31     ` Konstantin Khlebnikov
2012-02-23 13:51 ` [PATCH v3 03/21] memcg: fix page_referencies cgroup filter on global reclaim Konstantin Khlebnikov
2012-02-28  0:13   ` KAMEZAWA Hiroyuki
2012-02-23 13:51 ` [PATCH v3 04/21] memcg: use vm_swappiness from target memory cgroup Konstantin Khlebnikov
2012-02-28  0:15   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 05/21] mm: rename lruvec->lists into lruvec->pages_lru Konstantin Khlebnikov
2012-02-28  0:20   ` KAMEZAWA Hiroyuki
2012-02-28  6:04     ` Konstantin Khlebnikov
2012-02-23 13:52 ` [PATCH v3 06/21] mm: lruvec linking functions Konstantin Khlebnikov
2012-02-28  0:27   ` KAMEZAWA Hiroyuki
2012-02-28  6:09     ` Konstantin Khlebnikov
2012-02-23 13:52 ` [PATCH v3 07/21] mm: add lruvec->pages_count Konstantin Khlebnikov
2012-02-28  0:35   ` KAMEZAWA Hiroyuki
2012-02-28  6:16     ` Konstantin Khlebnikov
2012-02-23 13:52 ` [PATCH v3 08/21] mm: unify inactive_list_is_low() Konstantin Khlebnikov
2012-02-28  0:36   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 09/21] mm: add lruvec->reclaim_stat Konstantin Khlebnikov
2012-02-28  0:38   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 10/21] mm: kill struct mem_cgroup_zone Konstantin Khlebnikov
2012-02-28  0:41   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 11/21] mm: move page-to-lruvec translation upper Konstantin Khlebnikov
2012-02-28  0:42   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 12/21] mm: push lruvec into update_page_reclaim_stat() Konstantin Khlebnikov
2012-02-28  0:44   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 13/21] mm: push lruvecs from pagevec_lru_move_fn() to iterator Konstantin Khlebnikov
2012-02-28  0:45   ` KAMEZAWA Hiroyuki
2012-02-23 13:52 ` [PATCH v3 14/21] mm: introduce lruvec locking primitives Konstantin Khlebnikov
2012-02-28  0:56   ` KAMEZAWA Hiroyuki
2012-02-28  6:23     ` Konstantin Khlebnikov
2012-02-23 13:52 ` [PATCH v3 15/21] mm: handle lruvec relocks on lumpy reclaim Konstantin Khlebnikov
2012-02-28  1:01   ` KAMEZAWA Hiroyuki
2012-02-28  6:25     ` Konstantin Khlebnikov
2012-02-23 13:52 ` [PATCH v3 16/21] mm: handle lruvec relocks in compaction Konstantin Khlebnikov
2012-02-28  1:13   ` KAMEZAWA Hiroyuki
2012-02-28  6:31     ` Konstantin Khlebnikov
2012-02-23 13:53 ` [PATCH v3 17/21] mm: handle lruvec relock in memory controller Konstantin Khlebnikov
2012-02-28  1:22   ` KAMEZAWA Hiroyuki
2012-02-23 13:53 ` [PATCH v3 18/21] mm: add to lruvec isolated pages counters Konstantin Khlebnikov
2012-02-24  5:32   ` Konstantin Khlebnikov
2012-02-28  1:38   ` KAMEZAWA Hiroyuki
2012-02-23 13:53 ` [PATCH v3 19/21] memcg: check lru vectors emptiness in pre-destroy Konstantin Khlebnikov
2012-02-28  1:43   ` KAMEZAWA Hiroyuki
2012-02-23 13:53 ` [PATCH v3 20/21] mm: split zone->lru_lock Konstantin Khlebnikov
2012-02-28  1:49   ` KAMEZAWA Hiroyuki [this message]
2012-02-28  6:39     ` Konstantin Khlebnikov
2012-02-23 13:53 ` [PATCH v3 21/21] mm: zone lru vectors interleaving Konstantin Khlebnikov
2012-02-23 14:44   ` Hillf Danton
2012-02-23 16:21   ` Andi Kleen
2012-02-23 18:48     ` [PATCH 1/2] mm: configure lruvec split by boot options Konstantin Khlebnikov
2012-02-23 18:48     ` [PATCH 2/2] mm: show zone lruvec state in /proc/zoneinfo Konstantin Khlebnikov
2012-02-25  0:05 ` [PATCH v3 00/21] mm: lru_lock splitting Tim Chen
2012-02-25  5:34   ` Konstantin Khlebnikov
2012-02-25  2:15 ` KAMEZAWA Hiroyuki
2012-02-25  5:31   ` Konstantin Khlebnikov
2012-02-26 23:54     ` KAMEZAWA Hiroyuki
2012-02-28  1:52 ` KAMEZAWA Hiroyuki
2012-02-28  6:49   ` Konstantin Khlebnikov

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=20120228104913.0fc13bfd.kamezawa.hiroyu@jp.fujitsu.com \
    --to=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=khlebnikov@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /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