LinuxLists.cc - cgroup: real meaning of memory.usage_in

2011-03-18 15:25:42

Subject: cgroup: real meaning of memory.usage_in_bytes

Hi Kame,

I have received a report that our SLE11-SP1 (based on 2.6.32) kernel
doesn't pass LTP cgroup test case[*]. The test case basically creates a
cgroup (with 100M), runs a simple allocator which dirties a certain
amount of anonymous memory (under the limit) and finally checks whether
memory.usage_in_bytes == memory.stat (rss value).

This is obviously not 100% correct as the test should consider also
cache size but this test case doesn't end up using any cache pages so it
used worked when it was developed.

According to our documention this is a reasonable test case:
Documentation/cgroups/memory.txt:
memory.usage_in_bytes # show current memory(RSS+Cache) usage.

This however doesn't work after your commit:
cdec2e4265d (memcg: coalesce charging via percpu storage)

because since then we are charging in bulks so we can end up with
rss+cache <= usage_in_bytes. Simple (attached) program will
show this as well:
# mkdir /dev/memctl; mount -t cgroup -omemory cgroup /dev/memctl; cd /dev/memctl
# mkdir group_1; cd group_1; echo 100M > memory.limit_in_bytes
# cat memory.{usage_in_bytes,stat}
0
cache 0
rss 0
[...]

[run the program - it will print its pid and wait for enter]
echo pid > tasks

[hit enter to make the program mmap and dirty pages]
# cat memory.{usage_in_bytes,stat}
131072
cache 0
rss 4096
[...]

[hit enter again to let it finish]
# cat memory.{usage_in_bytes,stat}
126976
cache 0
rss 0
[...]

I think we have several options here
1) document that the value is actually >= rss+cache and it shows
the guaranteed charges for the group
2) use rss+cache rather then res->count
3) remove the file
4) call drain_all_stock_sync before asking for the value in
mem_cgroup_read
5) collect the current amount of stock charges and subtract it
from the current res->count value

1) and 2) would suggest that the file is actually not very much useful.
3) is basically the interface change as well
4) sounds little bit invasive as we basically lose the advantage of the
pool whenever somebody reads the file. Btw. for who is this file
intended?
5) sounds like a compromise

As I do not see a point of the file I would like to get rid of it
completely rather than play games around it but I am not sure why we
have it in the first place.

What do you (and others) think? I have a patch for 4 ready here but I
would like to understand the purpose of the file more before I post it.

Thanks
---
[*] You can get source at http://sourceforge.net/projects/ltp/
./testcases/kernel/controllers/memctl/memctl_test01.c and
./testcases/kernel/controllers/memctl/run_memctl_test.sh

The test should be executed with 4 as the parameter
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic

2011-03-18 15:29:44

Hi,

On Tue 29-03-11 09:21:22, Michal Hocko wrote:
> On Tue 29-03-11 10:15:11, Daisuke Nishimura wrote:
> [...]
> > From: Daisuke Nishimura <[email protected]>
> >
> > Since 569b846d(memcg: coalesce uncharge during unmap/truncate), we do batched
> > (delayed) uncharge at truncation/unmap. And since cdec2e42(memcg: coalesce
> > charging via percpu storage), we have percpu cache for res_counter.
> >
> > These changes improved performance of memory cgroup very much, but made
> > res_counter->usage usually have a bigger value than the actual value of memory usage.
> > So, *.usage_in_bytes, which show res_counter->usage, are not desirable for precise
> > values of memory(and swap) usage anymore.
> >
> > Instead of removing these files completely(because we cannot know res_counter->usage
> > without them), this patch updates the meaning of those files.
> >
> > Signed-off-by: Daisuke Nishimura <[email protected]>
> > ---
> > Documentation/cgroups/memory.txt | 15 +++++++++++++--
> > 1 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> > index 7781857..4f49d91 100644
> > --- a/Documentation/cgroups/memory.txt
> > +++ b/Documentation/cgroups/memory.txt
> > @@ -52,8 +52,10 @@ Brief summary of control files.
> > tasks # attach a task(thread) and show list of threads
> > cgroup.procs # show list of processes
> > cgroup.event_control # an interface for event_fd()
> > - memory.usage_in_bytes # show current memory(RSS+Cache) usage.
> > - memory.memsw.usage_in_bytes # show current memory+Swap usage
> > + memory.usage_in_bytes # show current res_counter usage for memory
> > + (See 5.5 for details)
> > + memory.memsw.usage_in_bytes # show current res_counter usage for memory+Swap
> > + (See 5.5 for details)
> > memory.limit_in_bytes # set/show limit of memory usage
> > memory.memsw.limit_in_bytes # set/show limit of memory+Swap usage
> > memory.failcnt # show the number of memory usage hits limits
> > @@ -453,6 +455,15 @@ memory under it will be reclaimed.
> > You can reset failcnt by writing 0 to failcnt file.
> > # echo 0 > .../memory.failcnt
> >
> > +5.5 usage_in_bytes
> > +
> > +For efficiency, as other kernel components, memory cgroup uses some optimization
> > +to avoid unnecessary cacheline false sharing. usage_in_bytes is affected by the
> > +method and doesn't show 'exact' value of memory(and swap) usage, it's an fuzz
> > +value for efficient access. (Of course, when necessary, it's synchronized.)
> > +If you want to know more exact memory usage, you should use RSS+CACHE(+SWAP)
> > +value in memory.stat(see 5.2).
> > +
> > 6. Hierarchy support
> >
> > The memory controller supports a deep hierarchy and hierarchical accounting.
>
> Acked-by: Michal Hocko <[email protected]>
>
> Although I would like to see a mention about what is the reason for
> keeping that file(s) if their usage is very limited. Something like.
> "We are keeping the file because we want to be consistent with other
> cgroups implementations and all of them export usage counter in some
> way. Make sure you exactly know the meaning before you use the value
> in userspace."
>
> If nobody else feels that this is that important then please forget
> about this comment.

I am wondering what happened to the patch. Do you have any plans to
update/push it?

--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic

2011-04-19 23:37:51

by Daisuke Nishimura

[permalink] [raw]

Subject: Re: [PATCH v2] memcg: update documentation to describe usage_in_bytes

Hi,

On Tue, 19 Apr 2011 14:14:14 +0200
Michal Hocko <[email protected]> wrote:

> Hi,
>
> On Tue 29-03-11 09:21:22, Michal Hocko wrote:
> > On Tue 29-03-11 10:15:11, Daisuke Nishimura wrote:
> > [...]
> > > From: Daisuke Nishimura <[email protected]>
> > >
> > > Since 569b846d(memcg: coalesce uncharge during unmap/truncate), we do batched
> > > (delayed) uncharge at truncation/unmap. And since cdec2e42(memcg: coalesce
> > > charging via percpu storage), we have percpu cache for res_counter.
> > >
> > > These changes improved performance of memory cgroup very much, but made
> > > res_counter->usage usually have a bigger value than the actual value of memory usage.
> > > So, *.usage_in_bytes, which show res_counter->usage, are not desirable for precise
> > > values of memory(and swap) usage anymore.
> > >
> > > Instead of removing these files completely(because we cannot know res_counter->usage
> > > without them), this patch updates the meaning of those files.
> > >
> > > Signed-off-by: Daisuke Nishimura <[email protected]>
> > > ---
> > > Documentation/cgroups/memory.txt | 15 +++++++++++++--
> > > 1 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> > > index 7781857..4f49d91 100644
> > > --- a/Documentation/cgroups/memory.txt
> > > +++ b/Documentation/cgroups/memory.txt
> > > @@ -52,8 +52,10 @@ Brief summary of control files.
> > > tasks # attach a task(thread) and show list of threads
> > > cgroup.procs # show list of processes
> > > cgroup.event_control # an interface for event_fd()
> > > - memory.usage_in_bytes # show current memory(RSS+Cache) usage.
> > > - memory.memsw.usage_in_bytes # show current memory+Swap usage
> > > + memory.usage_in_bytes # show current res_counter usage for memory
> > > + (See 5.5 for details)
> > > + memory.memsw.usage_in_bytes # show current res_counter usage for memory+Swap
> > > + (See 5.5 for details)
> > > memory.limit_in_bytes # set/show limit of memory usage
> > > memory.memsw.limit_in_bytes # set/show limit of memory+Swap usage
> > > memory.failcnt # show the number of memory usage hits limits
> > > @@ -453,6 +455,15 @@ memory under it will be reclaimed.
> > > You can reset failcnt by writing 0 to failcnt file.
> > > # echo 0 > .../memory.failcnt
> > >
> > > +5.5 usage_in_bytes
> > > +
> > > +For efficiency, as other kernel components, memory cgroup uses some optimization
> > > +to avoid unnecessary cacheline false sharing. usage_in_bytes is affected by the
> > > +method and doesn't show 'exact' value of memory(and swap) usage, it's an fuzz
> > > +value for efficient access. (Of course, when necessary, it's synchronized.)
> > > +If you want to know more exact memory usage, you should use RSS+CACHE(+SWAP)
> > > +value in memory.stat(see 5.2).
> > > +
> > > 6. Hierarchy support
> > >
> > > The memory controller supports a deep hierarchy and hierarchical accounting.
> >
> > Acked-by: Michal Hocko <[email protected]>
> >
> > Although I would like to see a mention about what is the reason for
> > keeping that file(s) if their usage is very limited. Something like.
> > "We are keeping the file because we want to be consistent with other
> > cgroups implementations and all of them export usage counter in some
> > way. Make sure you exactly know the meaning before you use the value
> > in userspace."
> >
> > If nobody else feels that this is that important then please forget
> > about this comment.
>
> I am wondering what happened to the patch. Do you have any plans to
> update/push it?
>
Ouch! Thank you for your reminding me..

I'll resend this to Andrew soon.

Thanks,
Daisuke Nishimura.