2008-03-11 15:02:23

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH] Add the max_usage member on the res_counter

This field is the maximal value of the usage one since the counter
creation (or since the latest reset).

To reset this to the usage value simply write anything to the
appropriate cgroup file.

Signed-off-by: Pavel Emelyanov <[email protected]>
Acked-by: Balbir Singh <[email protected]>

---

diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h
index 8cb1ecd..df8085a 100644
--- a/include/linux/res_counter.h
+++ b/include/linux/res_counter.h
@@ -25,6 +25,10 @@ struct res_counter {
*/
unsigned long long usage;
/*
+ * the maximal value of the usage from the counter creation
+ */
+ unsigned long long max_usage;
+ /*
* the limit that usage cannot exceed
*/
unsigned long long limit;
@@ -67,6 +71,7 @@ ssize_t res_counter_write(struct res_counter *counter, int member,

enum {
RES_USAGE,
+ RES_MAX_USAGE,
RES_LIMIT,
RES_FAILCNT,
};
@@ -127,4 +132,13 @@ static inline bool res_counter_check_under_limit(struct res_counter *cnt)
return ret;
}

+static inline void res_counter_reset_max(struct res_counter *cnt)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&cnt->lock, flags);
+ cnt->max_usage = cnt->usage;
+ spin_unlock_irqrestore(&cnt->lock, flags);
+}
+
#endif
diff --git a/kernel/res_counter.c b/kernel/res_counter.c
index 791ff2b..f1f20c2 100644
--- a/kernel/res_counter.c
+++ b/kernel/res_counter.c
@@ -27,6 +27,8 @@ int res_counter_charge_locked(struct res_counter *counter, unsigned long val)
}

counter->usage += val;
+ if (counter->usage > counter->max_usage)
+ counter->max_usage = counter->usage;
return 0;
}

@@ -65,6 +67,8 @@ res_counter_member(struct res_counter *counter, int member)
switch (member) {
case RES_USAGE:
return &counter->usage;
+ case RES_MAX_USAGE:
+ return &counter->max_usage;
case RES_LIMIT:
return &counter->limit;
case RES_FAILCNT:
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index eb681a6..c27141d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -868,6 +868,17 @@ static ssize_t mem_cgroup_write(struct cgroup *cont, struct cftype *cft,
mem_cgroup_write_strategy);
}

+static ssize_t mem_cgroup_max_reset(struct cgroup *cont, struct cftype *cft,
+ struct file *file, const char __user *userbuf,
+ size_t nbytes, loff_t *ppos)
+{
+ struct mem_cgroup *mem;
+
+ mem = mem_cgroup_from_cont(cont);
+ res_counter_reset_max(&mem->res);
+ return nbytes;
+}
+
static ssize_t mem_force_empty_write(struct cgroup *cont,
struct cftype *cft, struct file *file,
const char __user *userbuf,
@@ -923,6 +934,12 @@ static struct cftype mem_cgroup_files[] = {
.read_u64 = mem_cgroup_read,
},
{
+ .name = "max_usage_in_bytes",
+ .private = RES_MAX_USAGE,
+ .write = mem_cgroup_max_reset,
+ .read_u64 = mem_cgroup_read,
+ },
+ {
.name = "limit_in_bytes",
.private = RES_LIMIT,
.write = mem_cgroup_write,


2008-03-12 04:40:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Add the max_usage member on the res_counter

On Tue, 11 Mar 2008 17:55:46 +0300 Pavel Emelyanov <[email protected]> wrote:

> This field is the maximal value of the usage one since the counter
> creation (or since the latest reset).
>
> To reset this to the usage value simply write anything to the
> appropriate cgroup file.

What is the justification for making this change? Why do we need it?
Please provide this information for all patches where it is not obvious.

> {
> + .name = "max_usage_in_bytes",
> + .private = RES_MAX_USAGE,
> + .write = mem_cgroup_max_reset,
> + .read_u64 = mem_cgroup_read,
> + },

Documentation/controllers/memory.txt needs updating.

2008-03-12 09:05:16

by Pavel Emelyanov

[permalink] [raw]
Subject: Re: [PATCH] Add the max_usage member on the res_counter

Andrew Morton wrote:
> On Tue, 11 Mar 2008 17:55:46 +0300 Pavel Emelyanov <[email protected]> wrote:
>
>> This field is the maximal value of the usage one since the counter
>> creation (or since the latest reset).
>>
>> To reset this to the usage value simply write anything to the
>> appropriate cgroup file.
>
> What is the justification for making this change? Why do we need it?

The max_usage value is useful when gathering a statistical information
about the particular group, as it shows the actual memory requirements
for a particular group, not just some usage snapshot.

E.g. this value can be used to quickly tune the group. OpenVZ users often
set the limits to maximal values and either load the container with a common
patterns or leave one for a while. After this the max_usage value shows the
amount of memory the container would require during its common activity.
Setting the limit a bit above this value gives a pretty good configuration,
that works in the most cases.

But, please see my second comment.

> Please provide this information for all patches where it is not obvious.
>
>> {
>> + .name = "max_usage_in_bytes",
>> + .private = RES_MAX_USAGE,
>> + .write = mem_cgroup_max_reset,
>> + .read_u64 = mem_cgroup_read,
>> + },
>
> Documentation/controllers/memory.txt needs updating.
>

I'm preparing the Documentation/controllers/resource_counter.txt file
that will describe this abstraction. Since this one can be used for
many other resources (swap, kernel memory, networking, etc) this will
also eliminate all the possible future duplications in the appropriate
docs.

The above explanation of the max_usage attribute will be included in
this doc. Should it still be included in the previous patch's comment or
in the patch _as_a_ comment?