linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Balbir Singh <balbir@linux.vnet.ibm.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Ying Han <yinghan@google.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"kosaki.motohiro@jp.fujitsu.com" <kosaki.motohiro@jp.fujitsu.com>,
	"nishimura@mxp.nes.nec.co.jp" <nishimura@mxp.nes.nec.co.jp>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	Johannes Weiner <jweiner@redhat.com>,
	"minchan.kim@gmail.com" <minchan.kim@gmail.com>,
	Michal Hocko <mhocko@suse.cz>
Subject: Re: [PATCH 1/7] memcg: add high/low watermark to res_counter
Date: Mon, 2 May 2011 14:37:41 +0530	[thread overview]
Message-ID: <20110502090741.GP6547@balbir.in.ibm.com> (raw)
In-Reply-To: <20110425182849.ab708f12.kamezawa.hiroyu@jp.fujitsu.com>

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2011-04-25 18:28:49]:

> There are two watermarks added per-memcg including "high_wmark" and "low_wmark".
> The per-memcg kswapd is invoked when the memcg's memory usage(usage_in_bytes)
> is higher than the low_wmark. Then the kswapd thread starts to reclaim pages
> until the usage is lower than the high_wmark.
> 
> Each watermark is calculated based on the hard_limit(limit_in_bytes) for each
> memcg. Each time the hard_limit is changed, the corresponding wmarks are
> re-calculated. Since memory controller charges only user pages, there is
> no need for a "min_wmark". The current calculation of wmarks is based on
> individual tunable high_wmark_distance, which are set to 0 by default.
> low_wmark is calculated in automatic way.
> 
> Changelog:v8b...v7
> 1. set low_wmark_distance in automatic using fixed HILOW_DISTANCE.
> 
> Signed-off-by: Ying Han <yinghan@google.com>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
>  include/linux/memcontrol.h  |    1 
>  include/linux/res_counter.h |   78 ++++++++++++++++++++++++++++++++++++++++++++
>  kernel/res_counter.c        |    6 +++
>  mm/memcontrol.c             |   69 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 154 insertions(+)
> 
> Index: memcg/include/linux/memcontrol.h
> ===================================================================
> --- memcg.orig/include/linux/memcontrol.h
> +++ memcg/include/linux/memcontrol.h
> @@ -84,6 +84,7 @@ int task_in_mem_cgroup(struct task_struc
> 
>  extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
>  extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p);
> +extern int mem_cgroup_watermark_ok(struct mem_cgroup *mem, int charge_flags);
> 
>  static inline
>  int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup)
> Index: memcg/include/linux/res_counter.h
> ===================================================================
> --- memcg.orig/include/linux/res_counter.h
> +++ memcg/include/linux/res_counter.h
> @@ -39,6 +39,14 @@ struct res_counter {
>  	 */
>  	unsigned long long soft_limit;
>  	/*
> +	 * the limit that reclaim triggers.
> +	 */
> +	unsigned long long low_wmark_limit;
> +	/*
> +	 * the limit that reclaim stops.
> +	 */
> +	unsigned long long high_wmark_limit;
> +	/*
>  	 * the number of unsuccessful attempts to consume the resource
>  	 */
>  	unsigned long long failcnt;
> @@ -55,6 +63,9 @@ struct res_counter {
> 
>  #define RESOURCE_MAX (unsigned long long)LLONG_MAX
> 
> +#define CHARGE_WMARK_LOW	0x01
> +#define CHARGE_WMARK_HIGH	0x02
> +
>  /**
>   * Helpers to interact with userspace
>   * res_counter_read_u64() - returns the value of the specified member.
> @@ -92,6 +103,8 @@ enum {
>  	RES_LIMIT,
>  	RES_FAILCNT,
>  	RES_SOFT_LIMIT,
> +	RES_LOW_WMARK_LIMIT,
> +	RES_HIGH_WMARK_LIMIT
>  };
> 
>  /*
> @@ -147,6 +160,24 @@ static inline unsigned long long res_cou
>  	return margin;
>  }
> 
> +static inline bool
> +res_counter_under_high_wmark_limit_check_locked(struct res_counter *cnt)
> +{
> +	if (cnt->usage < cnt->high_wmark_limit)
> +		return true;
> +
> +	return false;
> +}
> +
> +static inline bool
> +res_counter_under_low_wmark_limit_check_locked(struct res_counter *cnt)
> +{
> +	if (cnt->usage < cnt->low_wmark_limit)
> +		return true;
> +
> +	return false;
> +}
> +
>  /**
>   * Get the difference between the usage and the soft limit
>   * @cnt: The counter
> @@ -169,6 +200,30 @@ res_counter_soft_limit_excess(struct res
>  	return excess;
>  }
> 
> +static inline bool
> +res_counter_under_low_wmark_limit(struct res_counter *cnt)
> +{
> +	bool ret;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cnt->lock, flags);
> +	ret = res_counter_under_low_wmark_limit_check_locked(cnt);
> +	spin_unlock_irqrestore(&cnt->lock, flags);
> +	return ret;
> +}
> +
> +static inline bool
> +res_counter_under_high_wmark_limit(struct res_counter *cnt)
> +{
> +	bool ret;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cnt->lock, flags);
> +	ret = res_counter_under_high_wmark_limit_check_locked(cnt);
> +	spin_unlock_irqrestore(&cnt->lock, flags);
> +	return ret;
> +}
> +
>  static inline void res_counter_reset_max(struct res_counter *cnt)
>  {
>  	unsigned long flags;
> @@ -214,4 +269,27 @@ res_counter_set_soft_limit(struct res_co
>  	return 0;
>  }
> 
> +static inline int
> +res_counter_set_high_wmark_limit(struct res_counter *cnt,
> +				unsigned long long wmark_limit)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cnt->lock, flags);
> +	cnt->high_wmark_limit = wmark_limit;
> +	spin_unlock_irqrestore(&cnt->lock, flags);
> +	return 0;
> +}
> +
> +static inline int
> +res_counter_set_low_wmark_limit(struct res_counter *cnt,
> +				unsigned long long wmark_limit)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&cnt->lock, flags);
> +	cnt->low_wmark_limit = wmark_limit;
> +	spin_unlock_irqrestore(&cnt->lock, flags);
> +	return 0;
> +}
>  #endif
> Index: memcg/kernel/res_counter.c
> ===================================================================
> --- memcg.orig/kernel/res_counter.c
> +++ memcg/kernel/res_counter.c
> @@ -19,6 +19,8 @@ void res_counter_init(struct res_counter
>  	spin_lock_init(&counter->lock);
>  	counter->limit = RESOURCE_MAX;
>  	counter->soft_limit = RESOURCE_MAX;
> +	counter->low_wmark_limit = RESOURCE_MAX;
> +	counter->high_wmark_limit = RESOURCE_MAX;
>  	counter->parent = parent;
>  }
> 
> @@ -103,6 +105,10 @@ res_counter_member(struct res_counter *c
>  		return &counter->failcnt;
>  	case RES_SOFT_LIMIT:
>  		return &counter->soft_limit;
> +	case RES_LOW_WMARK_LIMIT:
> +		return &counter->low_wmark_limit;
> +	case RES_HIGH_WMARK_LIMIT:
> +		return &counter->high_wmark_limit;
>  	};
> 
>  	BUG();
> Index: memcg/mm/memcontrol.c
> ===================================================================
> --- memcg.orig/mm/memcontrol.c
> +++ memcg/mm/memcontrol.c
> @@ -278,6 +278,11 @@ struct mem_cgroup {
>  	 */
>  	struct mem_cgroup_stat_cpu nocpu_base;
>  	spinlock_t pcp_counter_lock;
> +
> +	/*
> +	 * used to calculate the low/high_wmarks based on the limit_in_bytes.
> +	 */
> +	u64 high_wmark_distance;
>  };
> 
>  /* Stuffs for move charges at task migration. */
> @@ -867,6 +872,44 @@ out:
>  EXPORT_SYMBOL(mem_cgroup_count_vm_event);
>

Hmm... I wonder if we can start looking at the read side of
usage_in_bytes using RCU and reduce lock contention on cnt->lock. May
be an optimization for later. I still have my old per cpu counter
patches for usage_in_bytes that add some fuzz factor, but help improve
speed. I should rebase them and try.

 
>  /*
> + * If Hi-Low distance is too big, background reclaim tend to be cpu hogging.
> + * If Hi-Low distance is too small, small memory usage spike (by temporal
> + * shell scripts) causes background reclaim and make thing worse. But memory
> + * spike can be avoided by setting high-wmark a bit higier. We use fixed size
> + * size of HiLow Distance, this will be easy to use.
> + */
> +#ifdef CONFIG_64BIT /* object size tend do be twice */
> +#define HILOW_DISTANCE	(4 * 1024 * 1024)
> +#else
> +#define HILOW_DISTANCE	(2 * 1024 * 1024)
> +#endif
> +
> +static void setup_per_memcg_wmarks(struct mem_cgroup *mem)
> +{
> +	u64 limit;
> +
> +	limit = res_counter_read_u64(&mem->res, RES_LIMIT);
> +	if (mem->high_wmark_distance == 0) {
> +		res_counter_set_low_wmark_limit(&mem->res, limit);
> +		res_counter_set_high_wmark_limit(&mem->res, limit);
> +	} else {
> +		u64 low_wmark, high_wmark, low_distance;
> +		if (mem->high_wmark_distance <= HILOW_DISTANCE)
> +			low_distance = mem->high_wmark_distance / 2;
> +		else
> +			low_distance = HILOW_DISTANCE;
> +		if (low_distance < PAGE_SIZE * 2)
> +			low_distance = PAGE_SIZE * 2;
> +
> +		low_wmark = limit - low_distance;
> +		high_wmark = limit - mem->high_wmark_distance;
> +
> +		res_counter_set_low_wmark_limit(&mem->res, low_wmark);
> +		res_counter_set_high_wmark_limit(&mem->res, high_wmark);
> +	}
> +}
> +

I've not seen the documentation patch, but it might be good to have
some comments with what to expect the watermarks to be and who sets up
up high_wmark_distance. 

> +/*
>   * Following LRU functions are allowed to be used without PCG_LOCK.
>   * Operations are called by routine of global LRU independently from memcg.
>   * What we have to take care of here is validness of pc->mem_cgroup.
> @@ -3264,6 +3307,7 @@ static int mem_cgroup_resize_limit(struc
>  			else
>  				memcg->memsw_is_minimum = false;
>  		}
> +		setup_per_memcg_wmarks(memcg);
>  		mutex_unlock(&set_limit_mutex);
> 
>  		if (!ret)
> @@ -3324,6 +3368,7 @@ static int mem_cgroup_resize_memsw_limit
>  			else
>  				memcg->memsw_is_minimum = false;
>  		}
> +		setup_per_memcg_wmarks(memcg);
>  		mutex_unlock(&set_limit_mutex);
> 
>  		if (!ret)
> @@ -4603,6 +4648,30 @@ static void __init enable_swap_cgroup(vo
>  }
>  #endif
> 
> +/*
> + * We use low_wmark and high_wmark for triggering per-memcg kswapd.
> + * The reclaim is triggered by low_wmark (usage > low_wmark) and stopped
> + * by high_wmark (usage < high_wmark).
> + */
> +int mem_cgroup_watermark_ok(struct mem_cgroup *mem,
> +				int charge_flags)
> +{
> +	long ret = 0;
> +	int flags = CHARGE_WMARK_LOW | CHARGE_WMARK_HIGH;
> +
> +	if (!mem->high_wmark_distance)
> +		return 1;
> +
> +	VM_BUG_ON((charge_flags & flags) == flags);
> +
> +	if (charge_flags & CHARGE_WMARK_LOW)
> +		ret = res_counter_under_low_wmark_limit(&mem->res);
> +	if (charge_flags & CHARGE_WMARK_HIGH)
> +		ret = res_counter_under_high_wmark_limit(&mem->res);
> +
> +	return ret;
> +}
> +
>  static int mem_cgroup_soft_limit_tree_init(void)
>  {
>  	struct mem_cgroup_tree_per_node *rtpn;
> 
> --
> 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>

-- 
	Three Cheers,
	Balbir

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

  parent reply	other threads:[~2011-05-02 10:36 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-25  9:25 [PATCH 0/7] memcg background reclaim , yet another one KAMEZAWA Hiroyuki
2011-04-25  9:28 ` [PATCH 1/7] memcg: add high/low watermark to res_counter KAMEZAWA Hiroyuki
2011-04-26 17:54   ` Ying Han
2011-04-29 13:33   ` Michal Hocko
2011-05-01  6:06     ` KOSAKI Motohiro
2011-05-03  6:49       ` Michal Hocko
2011-05-03  7:45         ` KOSAKI Motohiro
2011-05-03  8:25           ` Michal Hocko
2011-05-03 17:01             ` Ying Han
2011-05-04  8:58               ` Michal Hocko
2011-05-04 17:16                 ` Ying Han
2011-05-05  6:59                   ` Michal Hocko
2011-05-06  5:28                     ` KAMEZAWA Hiroyuki
2011-05-06 14:22                       ` Johannes Weiner
2011-05-09  0:21                         ` KAMEZAWA Hiroyuki
2011-05-09  5:47                           ` Ying Han
2011-05-09  9:58                           ` Johannes Weiner
2011-05-09  9:59                             ` KAMEZAWA Hiroyuki
2011-05-10  4:43                             ` Ying Han
2011-05-09  5:40                       ` Ying Han
2011-05-09  7:10                         ` KAMEZAWA Hiroyuki
2011-05-09 10:18                           ` Johannes Weiner
2011-05-09 12:49                             ` Michal Hocko
2011-05-09 23:49                               ` KAMEZAWA Hiroyuki
2011-05-10  4:39                                 ` Ying Han
2011-05-10  4:51                             ` Ying Han
2011-05-10  6:27                               ` Johannes Weiner
2011-05-10  7:09                                 ` Ying Han
2011-05-04  3:55             ` KOSAKI Motohiro
2011-05-04  8:55               ` Michal Hocko
2011-05-09  3:24                 ` KOSAKI Motohiro
2011-05-02  9:07   ` Balbir Singh [this message]
2011-05-06  5:30     ` KAMEZAWA Hiroyuki
2011-04-25  9:29 ` [PATCH 2/7] memcg high watermark interface KAMEZAWA Hiroyuki
2011-04-25 22:36   ` Ying Han
2011-04-25  9:31 ` [PATCH 3/7] memcg: select victim node in round robin KAMEZAWA Hiroyuki
2011-04-25  9:34 ` [PATCH 4/7] memcg fix scan ratio with small memcg KAMEZAWA Hiroyuki
2011-04-25 17:35   ` Ying Han
2011-04-26  1:43     ` KAMEZAWA Hiroyuki
2011-04-25  9:36 ` [PATCH 5/7] memcg bgreclaim core KAMEZAWA Hiroyuki
2011-04-26  4:59   ` Ying Han
2011-04-26  5:08     ` KAMEZAWA Hiroyuki
2011-04-26 23:15       ` Ying Han
2011-04-27  0:10         ` KAMEZAWA Hiroyuki
2011-04-27  1:01           ` KAMEZAWA Hiroyuki
2011-04-26 18:37   ` Ying Han
2011-04-25  9:40 ` [PATCH 6/7] memcg add zone_all_unreclaimable KAMEZAWA Hiroyuki
2011-04-25  9:42 ` [PATCH 7/7] memcg watermark reclaim workqueue KAMEZAWA Hiroyuki
2011-04-26 23:19   ` Ying Han
2011-04-27  0:31     ` KAMEZAWA Hiroyuki
2011-04-27  3:40       ` Ying Han
2011-04-25  9:43 ` [PATCH 8/7] memcg : reclaim statistics KAMEZAWA Hiroyuki
2011-04-26  5:35   ` Ying Han
2011-04-25  9:49 ` [PATCH 0/7] memcg background reclaim , yet another one KAMEZAWA Hiroyuki
2011-04-25 10:14 ` KAMEZAWA Hiroyuki
2011-04-25 22:21   ` Ying Han
2011-04-26  1:38     ` KAMEZAWA Hiroyuki
2011-04-26  7:19       ` Ying Han
2011-04-26  7:43         ` KAMEZAWA Hiroyuki
2011-04-26  8:43           ` Ying Han
2011-04-26  8:47             ` KAMEZAWA Hiroyuki
2011-04-26 23:08               ` Ying Han
2011-04-27  0:34                 ` KAMEZAWA Hiroyuki
2011-04-27  1:19                   ` Ying Han
2011-04-28  3:55               ` Ying Han
2011-04-28  4:05                 ` KAMEZAWA Hiroyuki
2011-05-02  7:02     ` Balbir Singh
2011-05-02  6:09 ` Balbir Singh

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=20110502090741.GP6547@balbir.in.ibm.com \
    --to=balbir@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=jweiner@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=minchan.kim@gmail.com \
    --cc=nishimura@mxp.nes.nec.co.jp \
    --cc=yinghan@google.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;
as well as URLs for NNTP newsgroup(s).