Re: [PATCH V4 03/10] New APIs to adjust per-memcg wmarks

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Thu, 14 Apr 2011 15:54:22 -0700
Ying Han <yinghan@google.com> wrote:

> Add memory.low_wmark_distance, memory.high_wmark_distance and reclaim_wmarks
> APIs per-memcg. The first two adjust the internal low/high wmark calculation
> and the reclaim_wmarks exports the current value of watermarks.
> 
> By default, the low/high_wmark is calculated by subtracting the distance from
> the hard_limit(limit_in_bytes).
> 
> $ echo 500m >/dev/cgroup/A/memory.limit_in_bytes
> $ cat /dev/cgroup/A/memory.limit_in_bytes
> 524288000
> 
> $ echo 50m >/dev/cgroup/A/memory.high_wmark_distance
> $ echo 40m >/dev/cgroup/A/memory.low_wmark_distance
> 
> $ cat /dev/cgroup/A/memory.reclaim_wmarks
> low_wmark 482344960
> high_wmark 471859200
> 
> changelog v4..v3:
> 1. replace the "wmark_ratio" API with individual tunable for low/high_wmarks.
> 
> changelog v3..v2:
> 1. replace the "min_free_kbytes" api with "wmark_ratio". This is part of
> feedbacks
> 
> Signed-off-by: Ying Han <yinghan@google.com>

Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

But please add a sanity check (see below.)



> ---
>  mm/memcontrol.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 95 insertions(+), 0 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1ec4014..685645c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3974,6 +3974,72 @@ static int mem_cgroup_swappiness_write(struct cgroup *cgrp, struct cftype *cft,
>  	return 0;
>  }
>  
> +static u64 mem_cgroup_high_wmark_distance_read(struct cgroup *cgrp,
> +					       struct cftype *cft)
> +{
> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp);
> +
> +	return memcg->high_wmark_distance;
> +}
> +
> +static u64 mem_cgroup_low_wmark_distance_read(struct cgroup *cgrp,
> +					      struct cftype *cft)
> +{
> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp);
> +
> +	return memcg->low_wmark_distance;
> +}
> +
> +static int mem_cgroup_high_wmark_distance_write(struct cgroup *cont,
> +						struct cftype *cft,
> +						const char *buffer)
> +{
> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
> +	u64 low_wmark_distance = memcg->low_wmark_distance;
> +	unsigned long long val;
> +	u64 limit;
> +	int ret;
> +
> +	ret = res_counter_memparse_write_strategy(buffer, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> +	if ((val >= limit) || (val < low_wmark_distance) ||
> +	   (low_wmark_distance && val == low_wmark_distance))
> +		return -EINVAL;
> +
> +	memcg->high_wmark_distance = val;
> +
> +	setup_per_memcg_wmarks(memcg);
> +	return 0;
> +}

IIUC, as limit_in_bytes, 'distance' should not be able to set against ROOT memcg
because it doesn't work.



> +
> +static int mem_cgroup_low_wmark_distance_write(struct cgroup *cont,
> +					       struct cftype *cft,
> +					       const char *buffer)
> +{
> +	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
> +	u64 high_wmark_distance = memcg->high_wmark_distance;
> +	unsigned long long val;
> +	u64 limit;
> +	int ret;
> +
> +	ret = res_counter_memparse_write_strategy(buffer, &val);
> +	if (ret)
> +		return -EINVAL;
> +
> +	limit = res_counter_read_u64(&memcg->res, RES_LIMIT);
> +	if ((val >= limit) || (val > high_wmark_distance) ||
> +	    (high_wmark_distance && val == high_wmark_distance))
> +		return -EINVAL;
> +
> +	memcg->low_wmark_distance = val;
> +
> +	setup_per_memcg_wmarks(memcg);
> +	return 0;
> +}
> +

Here, too.

I wonder we should have a method to hide unnecessary interfaces in ROOT cgroup...

Thanks,
-Kame

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