From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail202.messagelabs.com (mail202.messagelabs.com [216.82.254.227]) by kanga.kvack.org (Postfix) with ESMTP id 3534A8D003B for ; Tue, 19 Apr 2011 23:40:01 -0400 (EDT) Received: from wpaz5.hot.corp.google.com (wpaz5.hot.corp.google.com [172.24.198.69]) by smtp-out.google.com with ESMTP id p3K3dn5E001058 for ; Tue, 19 Apr 2011 20:39:49 -0700 Received: from qwj9 (qwj9.prod.google.com [10.241.195.73]) by wpaz5.hot.corp.google.com with ESMTP id p3K3dghT031983 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Tue, 19 Apr 2011 20:39:48 -0700 Received: by qwj9 with SMTP id 9so263358qwj.35 for ; Tue, 19 Apr 2011 20:39:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110420101533.d19622ce.kamezawa.hiroyu@jp.fujitsu.com> References: <1303185466-2532-1-git-send-email-yinghan@google.com> <1303185466-2532-10-git-send-email-yinghan@google.com> <20110420101533.d19622ce.kamezawa.hiroyu@jp.fujitsu.com> Date: Tue, 19 Apr 2011 20:39:47 -0700 Message-ID: Subject: Re: [PATCH V6 09/10] Add API to export per-memcg kswapd pid. From: Ying Han Content-Type: multipart/alternative; boundary=000e0cd68ee05b40af04a15160b1 Sender: owner-linux-mm@kvack.org List-ID: To: KAMEZAWA Hiroyuki Cc: KOSAKI Motohiro , Minchan Kim , Daisuke Nishimura , Balbir Singh , Tejun Heo , Pavel Emelyanov , Andrew Morton , Li Zefan , Mel Gorman , Christoph Lameter , Johannes Weiner , Rik van Riel , Hugh Dickins , Michal Hocko , Dave Hansen , Zhu Yanhai , linux-mm@kvack.org --000e0cd68ee05b40af04a15160b1 Content-Type: text/plain; charset=ISO-8859-1 On Tue, Apr 19, 2011 at 6:15 PM, KAMEZAWA Hiroyuki < kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Mon, 18 Apr 2011 20:57:45 -0700 > Ying Han wrote: > > > This add the API which exports per-memcg kswapd thread pid. The kswapd > > thread is named as "memcg_" + css_id, and the pid can be used to put > > kswapd thread into cpu cgroup later. > > > > $ mkdir /dev/cgroup/memory/A > > $ cat /dev/cgroup/memory/A/memory.kswapd_pid > > memcg_null 0 > > > > $ echo 500m >/dev/cgroup/memory/A/memory.limit_in_bytes > > $ echo 50m >/dev/cgroup/memory/A/memory.high_wmark_distance > > $ ps -ef | grep memcg > > root 6727 2 0 14:32 ? 00:00:00 [memcg_3] > > root 6729 6044 0 14:32 ttyS0 00:00:00 grep memcg > > > > $ cat memory.kswapd_pid > > memcg_3 6727 > > > > changelog v6..v5 > > 1. Remove the legacy spinlock which has been removed from previous post. > > > > changelog v5..v4 > > 1. Initialize the memcg-kswapd pid to -1 instead of 0. > > 2. Remove the kswapds_spinlock. > > > > changelog v4..v3 > > 1. Add the API based on KAMAZAWA's request on patch v3. > > > > Reviewed-by: KAMEZAWA Hiroyuki > > Signed-off-by: Ying Han > > I'm very sorry but please drop this. There is a discussion that > we should use thread pool rather than one-thread-per-one-memcg. > If so, we need to remove this interface and we'll see regression. > > I think we need some control knobs as priority/share in thread pools > finally... > (So, I want to use cpu cgroup.) If not, there will be unfair utilization of > cpu/thread. But for now, it seems adding this is too early. > This patch is is very good self-contained and i have no problem to drop it for now. And I won't include this in my next post. --Ying > > > > --- > > mm/memcontrol.c | 31 +++++++++++++++++++++++++++++++ > > 1 files changed, 31 insertions(+), 0 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index d5b284c..0b108b9 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -4533,6 +4533,33 @@ static int mem_cgroup_wmark_read(struct cgroup > *cgrp, > > return 0; > > } > > > > +static int mem_cgroup_kswapd_pid_read(struct cgroup *cgrp, > > + struct cftype *cft, struct cgroup_map_cb *cb) > > +{ > > + struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp); > > + struct task_struct *kswapd_thr = NULL; > > + struct kswapd *kswapd_p = NULL; > > + wait_queue_head_t *wait; > > + char name[TASK_COMM_LEN]; > > + pid_t pid = -1; > > + > > + sprintf(name, "memcg_null"); > > + > > + wait = mem_cgroup_kswapd_wait(mem); > > + if (wait) { > > + kswapd_p = container_of(wait, struct kswapd, kswapd_wait); > > + kswapd_thr = kswapd_p->kswapd_task; > > + if (kswapd_thr) { > > + get_task_comm(name, kswapd_thr); > > + pid = kswapd_thr->pid; > > + } > > + } > > + > > + cb->fill(cb, name, pid); > > + > > + return 0; > > +} > > + > > static int mem_cgroup_oom_control_read(struct cgroup *cgrp, > > struct cftype *cft, struct cgroup_map_cb *cb) > > { > > @@ -4650,6 +4677,10 @@ static struct cftype mem_cgroup_files[] = { > > .name = "reclaim_wmarks", > > .read_map = mem_cgroup_wmark_read, > > }, > > + { > > + .name = "kswapd_pid", > > + .read_map = mem_cgroup_kswapd_pid_read, > > + }, > > }; > > > > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP > > -- > > 1.7.3.1 > > > > > > --000e0cd68ee05b40af04a15160b1 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Tue, Apr 19, 2011 at 6:15 PM, KAMEZAW= A Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
On Mon, 18 Apr 2011 20:57:45 -0700
Ying Han <yinghan@google.com&g= t; wrote:

> This add the API which exports per-memcg kswapd thread pid. The kswapd=
> thread is named as "memcg_" + css_id, and the pid can be use= d to put
> kswapd thread into cpu cgroup later.
>
> $ mkdir /dev/cgroup/memory/A
> $ cat /dev/cgroup/memory/A/memory.kswapd_pid
> memcg_null 0
>
> $ echo 500m >/dev/cgroup/memory/A/memory.limit_in_bytes
> $ echo 50m >/dev/cgroup/memory/A/memory.high_wmark_distance
> $ ps -ef | grep memcg
> root =A0 =A0 =A06727 =A0 =A0 2 =A00 14:32 ? =A0 =A0 =A0 =A000:00:00 [m= emcg_3]
> root =A0 =A0 =A06729 =A06044 =A00 14:32 ttyS0 =A0 =A000:00:00 grep mem= cg
>
> $ cat memory.kswapd_pid
> memcg_3 6727
>
> changelog v6..v5
> 1. Remove the legacy spinlock which has been removed from previous pos= t.
>
> changelog v5..v4
> 1. Initialize the memcg-kswapd pid to -1 instead of 0.
> 2. Remove the kswapds_spinlock.
>
> changelog v4..v3
> 1. Add the API based on KAMAZAWA's request on patch v3.
>
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Ying Han <ying= han@google.com>

I'm very sorry but please drop this. There is a discussion that we should use thread pool rather than one-thread-per-one-memcg.
If so, we need to remove this interface and we'll see regression.

I think we need some control knobs as priority/share in thread pools finall= y...
(So, I want to use cpu cgroup.) If not, there will be unfair utilization of=
cpu/thread. But for now, it seems adding this is too early.

This patch is is very good self-contained and i have n= o problem to drop it for now. And I won't include this in my next post.=

--Ying=A0


> ---
> =A0mm/memcontrol.c | =A0 31 +++++++++++++++++++++++++++++++
> =A01 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index d5b284c..0b108b9 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4533,6 +4533,33 @@ static int mem_cgroup_wmark_read(struct cgroup = *cgrp,
> =A0 =A0 =A0 return 0;
> =A0}
>
> +static int mem_cgroup_kswapd_pid_read(struct cgroup *cgrp,
> + =A0 =A0 struct cftype *cft, =A0struct cgroup_map_cb *cb)
> +{
> + =A0 =A0 struct mem_cgroup *mem =3D mem_cgroup_from_cont(cgrp);
> + =A0 =A0 struct task_struct *kswapd_thr =3D NULL;
> + =A0 =A0 struct kswapd *kswapd_p =3D NULL;
> + =A0 =A0 wait_queue_head_t *wait;
> + =A0 =A0 char name[TASK_COMM_LEN];
> + =A0 =A0 pid_t pid =3D -1;
> +
> + =A0 =A0 sprintf(name, "memcg_null");
> +
> + =A0 =A0 wait =3D mem_cgroup_kswapd_wait(mem);
> + =A0 =A0 if (wait) {
> + =A0 =A0 =A0 =A0 =A0 =A0 kswapd_p =3D container_of(wait, struct kswap= d, kswapd_wait);
> + =A0 =A0 =A0 =A0 =A0 =A0 kswapd_thr =3D kswapd_p->kswapd_task;
> + =A0 =A0 =A0 =A0 =A0 =A0 if (kswapd_thr) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 get_task_comm(name, kswapd_t= hr);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pid =3D kswapd_thr->pid;<= br> > + =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 }
> +
> + =A0 =A0 cb->fill(cb, name, pid);
> +
> + =A0 =A0 return 0;
> +}
> +
> =A0static int mem_cgroup_oom_control_read(struct cgroup *cgrp,
> =A0 =A0 =A0 struct cftype *cft, =A0struct cgroup_map_cb *cb)
> =A0{
> @@ -4650,6 +4677,10 @@ static struct cftype mem_cgroup_files[] =3D { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 .name =3D "reclaim_wmarks",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .read_map =3D mem_cgroup_wmark_read,
> =A0 =A0 =A0 },
> + =A0 =A0 {
> + =A0 =A0 =A0 =A0 =A0 =A0 .name =3D "kswapd_pid",
> + =A0 =A0 =A0 =A0 =A0 =A0 .read_map =3D mem_cgroup_kswapd_pid_read, > + =A0 =A0 },
> =A0};
>
> =A0#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> --
> 1.7.3.1
>
>


--000e0cd68ee05b40af04a15160b1-- -- 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: email@kvack.org