2009-12-09 15:49:11

by Kirill A. Shutemov

[permalink] [raw]
Subject: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

We really want to take MEM_CGROUP_STAT_SWAPOUT into account.

Signed-off-by: Kirill A. Shutemov <[email protected]>
Cc: [email protected]
---
mm/memcontrol.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f99f599..6314015 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2541,6 +2541,7 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
val += idx_val;
mem_cgroup_get_recursive_idx_stat(mem,
MEM_CGROUP_STAT_SWAPOUT, &idx_val);
+ val += idx_val;
val <<= PAGE_SHIFT;
} else
val = res_counter_read_u64(&mem->memsw, name);
--
1.6.5.3


2009-12-09 16:12:47

by Daisuke Nishimura

[permalink] [raw]
Subject: Re: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

(Added Cc: Andrew Morton <[email protected]>)

On Wed, 9 Dec 2009 17:48:58 +0200
"Kirill A. Shutemov" <[email protected]> wrote:

> We really want to take MEM_CGROUP_STAT_SWAPOUT into account.
>
Nice catch!

Reviewed-by: Daisuke Nishimura <[email protected]>

> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Cc: [email protected]
> ---
> mm/memcontrol.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f99f599..6314015 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2541,6 +2541,7 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
> val += idx_val;
> mem_cgroup_get_recursive_idx_stat(mem,
> MEM_CGROUP_STAT_SWAPOUT, &idx_val);
> + val += idx_val;
> val <<= PAGE_SHIFT;
> } else
> val = res_counter_read_u64(&mem->memsw, name);

Regards,
Daisuke Nishimura.

2009-12-10 00:02:36

by Kamezawa Hiroyuki

[permalink] [raw]
Subject: Re: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

On Wed, 9 Dec 2009 17:48:58 +0200
"Kirill A. Shutemov" <[email protected]> wrote:

> We really want to take MEM_CGROUP_STAT_SWAPOUT into account.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Cc: [email protected]

Thanks.

Acked-by: KAMEZAWA Hiroyuki <[email protected]>

2009-12-10 00:22:14

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

On Thu, 10 Dec 2009 08:59:29 +0900
KAMEZAWA Hiroyuki <[email protected]> wrote:

> On Wed, 9 Dec 2009 17:48:58 +0200
> "Kirill A. Shutemov" <[email protected]> wrote:
>
> > We really want to take MEM_CGROUP_STAT_SWAPOUT into account.
> >
> > Signed-off-by: Kirill A. Shutemov <[email protected]>
> > Cc: [email protected]
>
> Thanks.
>
> Acked-by: KAMEZAWA Hiroyuki <[email protected]>
>

Is this bug sufficiently serious to justify a -stable backport?

If so, why?

Thanks.

2009-12-10 01:16:56

by Kamezawa Hiroyuki

[permalink] [raw]
Subject: Re: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

On Wed, 9 Dec 2009 16:21:09 -0800
Andrew Morton <[email protected]> wrote:

> On Thu, 10 Dec 2009 08:59:29 +0900
> KAMEZAWA Hiroyuki <[email protected]> wrote:
>
> > On Wed, 9 Dec 2009 17:48:58 +0200
> > "Kirill A. Shutemov" <[email protected]> wrote:
> >
> > > We really want to take MEM_CGROUP_STAT_SWAPOUT into account.
> > >
> > > Signed-off-by: Kirill A. Shutemov <[email protected]>
> > > Cc: [email protected]
> >
> > Thanks.
> >
> > Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> >
>
> Is this bug sufficiently serious to justify a -stable backport?
>
I think so.

> If so, why?
>

memory cgroup has a file memory.memsw.usage_in_bytes file. It shows sum of
the usage of pages and swapents in the cgroup. Now, root cgroup's
memsw.usage_in_bytes shows wrong value....the number of swapents are not
added. This patch fixesi it.

Thanks,
-Kame

2009-12-10 01:31:06

by Daisuke Nishimura

[permalink] [raw]
Subject: Re: [PATCH] [TRIVIAL] memcg: fix memory.memsw.usage_in_bytes for root cgroup

On Wed, 9 Dec 2009 16:21:09 -0800, Andrew Morton <[email protected]> wrote:
> On Thu, 10 Dec 2009 08:59:29 +0900
> KAMEZAWA Hiroyuki <[email protected]> wrote:
>
> > On Wed, 9 Dec 2009 17:48:58 +0200
> > "Kirill A. Shutemov" <[email protected]> wrote:
> >
> > > We really want to take MEM_CGROUP_STAT_SWAPOUT into account.
> > >
> > > Signed-off-by: Kirill A. Shutemov <[email protected]>
> > > Cc: [email protected]
> >
> > Thanks.
> >
> > Acked-by: KAMEZAWA Hiroyuki <[email protected]>
> >
>
> Is this bug sufficiently serious to justify a -stable backport?
>
> If so, why?
>
Well, the value of <root cgroup>/memory.memsw.usage_in_bytes would be incorrect
(swap usage would not be counted) without this patch. So the impact of this bug
depends on how the value is used.

Anyway, this bug exists only in 2.6.32 and this patch can be applied onto it
without any change.


Thanks,
Daisuke Nishimura.