Re: [PATCH 08/10] memcg: add cgroupfs interface to memcg dirtylimits
From: Andrea Righi
Date: Tue Oct 05 2010 - 05:28:59 EST
On Tue, Oct 05, 2010 at 12:33:15AM -0700, Greg Thelen wrote:
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> writes:
>
> > On Sun, 3 Oct 2010 23:58:03 -0700
> > Greg Thelen <gthelen@xxxxxxxxxx> wrote:
> >
> >> Add cgroupfs interface to memcg dirty page limits:
> >> Direct write-out is controlled with:
> >> - memory.dirty_ratio
> >> - memory.dirty_bytes
> >>
> >> Background write-out is controlled with:
> >> - memory.dirty_background_ratio
> >> - memory.dirty_background_bytes
> >>
> >> Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx>
> >> Signed-off-by: Greg Thelen <gthelen@xxxxxxxxxx>
> >
> > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >
> > a question below.
> >
> >
> >> ---
> >> mm/memcontrol.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 files changed, 89 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> >> index 6ec2625..2d45a0a 100644
> >> --- a/mm/memcontrol.c
> >> +++ b/mm/memcontrol.c
> >> @@ -100,6 +100,13 @@ enum mem_cgroup_stat_index {
> >> MEM_CGROUP_STAT_NSTATS,
> >> };
> >>
> >> +enum {
> >> + MEM_CGROUP_DIRTY_RATIO,
> >> + MEM_CGROUP_DIRTY_BYTES,
> >> + MEM_CGROUP_DIRTY_BACKGROUND_RATIO,
> >> + MEM_CGROUP_DIRTY_BACKGROUND_BYTES,
> >> +};
> >> +
> >> struct mem_cgroup_stat_cpu {
> >> s64 count[MEM_CGROUP_STAT_NSTATS];
> >> };
> >> @@ -4292,6 +4299,64 @@ static int mem_cgroup_oom_control_write(struct cgroup *cgrp,
> >> return 0;
> >> }
> >>
> >> +static u64 mem_cgroup_dirty_read(struct cgroup *cgrp, struct cftype *cft)
> >> +{
> >> + struct mem_cgroup *mem = mem_cgroup_from_cont(cgrp);
> >> + bool root;
> >> +
> >> + root = mem_cgroup_is_root(mem);
> >> +
> >> + switch (cft->private) {
> >> + case MEM_CGROUP_DIRTY_RATIO:
> >> + return root ? vm_dirty_ratio : mem->dirty_param.dirty_ratio;
> >> + case MEM_CGROUP_DIRTY_BYTES:
> >> + return root ? vm_dirty_bytes : mem->dirty_param.dirty_bytes;
> >> + case MEM_CGROUP_DIRTY_BACKGROUND_RATIO:
> >> + return root ? dirty_background_ratio :
> >> + mem->dirty_param.dirty_background_ratio;
> >> + case MEM_CGROUP_DIRTY_BACKGROUND_BYTES:
> >> + return root ? dirty_background_bytes :
> >> + mem->dirty_param.dirty_background_bytes;
> >> + default:
> >> + BUG();
> >> + }
> >> +}
> >> +
> >> +static int
> >> +mem_cgroup_dirty_write(struct cgroup *cgrp, struct cftype *cft, u64 val)
> >> +{
> >> + struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp);
> >> + int type = cft->private;
> >> +
> >> + if (cgrp->parent == NULL)
> >> + return -EINVAL;
> >> + if ((type == MEM_CGROUP_DIRTY_RATIO ||
> >> + type == MEM_CGROUP_DIRTY_BACKGROUND_RATIO) && val > 100)
> >> + return -EINVAL;
> >> + switch (type) {
> >> + case MEM_CGROUP_DIRTY_RATIO:
> >> + memcg->dirty_param.dirty_ratio = val;
> >> + memcg->dirty_param.dirty_bytes = 0;
> >> + break;
> >> + case MEM_CGROUP_DIRTY_BYTES:
> >> + memcg->dirty_param.dirty_bytes = val;
> >> + memcg->dirty_param.dirty_ratio = 0;
> >> + break;
> >> + case MEM_CGROUP_DIRTY_BACKGROUND_RATIO:
> >> + memcg->dirty_param.dirty_background_ratio = val;
> >> + memcg->dirty_param.dirty_background_bytes = 0;
> >> + break;
> >> + case MEM_CGROUP_DIRTY_BACKGROUND_BYTES:
> >> + memcg->dirty_param.dirty_background_bytes = val;
> >> + memcg->dirty_param.dirty_background_ratio = 0;
> >> + break;
> >
> >
> > Curious....is this same behavior as vm_dirty_ratio ?
>
> I think this is same behavior as vm_dirty_ratio. When vm_dirty_ratio is
> changed then dirty_ratio_handler() will set vm_dirty_bytes=0. When
> vm_dirty_bytes is written dirty_bytes_handler() will set
> vm_dirty_ratio=0. So I think that the per-memcg dirty memory parameters
> mimic the behavior of vm_dirty_ratio, vm_dirty_bytes and the other
> global dirty parameters.
>
> Am I missing your question?
mmh... looking at the code it seems the same behaviour, but in
Documentation/sysctl/vm.txt we say a different thing (i.e., for
dirty_bytes):
"If dirty_bytes is written, dirty_ratio becomes a function of its value
(dirty_bytes / the amount of dirtyable system memory)."
However, in dirty_bytes_handler()/dirty_ratio_handler() we actually set
the counterpart value as 0.
I think we should clarify the documentation.
Signed-off-by: Andrea Righi <arighi@xxxxxxxxxxx>
---
Documentation/sysctl/vm.txt | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index b606c2c..30289fa 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -80,8 +80,10 @@ dirty_background_bytes
Contains the amount of dirty memory at which the pdflush background writeback
daemon will start writeback.
-If dirty_background_bytes is written, dirty_background_ratio becomes a function
-of its value (dirty_background_bytes / the amount of dirtyable system memory).
+Note: dirty_background_bytes is the counterpart of dirty_background_ratio. Only
+one of them may be specified at a time. When one sysctl is written it is
+immediately taken into account to evaluate the dirty memory limits and the
+other appears as 0 when read.
==============================================================
@@ -97,8 +99,10 @@ dirty_bytes
Contains the amount of dirty memory at which a process generating disk writes
will itself start writeback.
-If dirty_bytes is written, dirty_ratio becomes a function of its value
-(dirty_bytes / the amount of dirtyable system memory).
+Note: dirty_bytes is the counterpart of dirty_ratio. Only one of them may be
+specified at a time. When one sysctl is written it is immediately taken into
+account to evaluate the dirty memory limits and the other appears as 0 when
+read.
Note: the minimum value allowed for dirty_bytes is two pages (in bytes); any
value lower than this limit will be ignored and the old configuration will be
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/