lkml.org 
[lkml]   [2008]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/6] res_counter: handle limit change
On Fri, Jun 13, 2008 at 2:29 AM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> Add a support to shrink_usage_at_limit_change feature to res_counter.
> memcg will use this to drop pages.

Sorry for the delay in looking at this.

I think the basic idea is great.

>
> Change log: xxx -> v4 (new file.)
> - cut out the limit-change part from hierarchy patch set.
> - add "retry_count" arguments to shrink_usage(). This allows that we don't
> have to set the default retry loop count.
> - res_counter_check_under_val() is added to support subsystem.
> - res_counter_init() is res_counter_init_ops(cnt, NULL)
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> ---
> Documentation/controllers/resource_counter.txt | 19 +++++-
> include/linux/res_counter.h | 33 ++++++++++-
> kernel/res_counter.c | 74 ++++++++++++++++++++++++-
> 3 files changed, 121 insertions(+), 5 deletions(-)
>
> Index: linux-2.6.26-rc5-mm3/include/linux/res_counter.h
> ===================================================================
> --- linux-2.6.26-rc5-mm3.orig/include/linux/res_counter.h
> +++ linux-2.6.26-rc5-mm3/include/linux/res_counter.h
> @@ -21,6 +21,13 @@
> * the helpers described beyond
> */
>
> +struct res_counter;
> +struct res_counter_ops {
> + /* called when the subsystem has to reduce the usage. */
> + int (*shrink_usage)(struct res_counter *cnt, unsigned long long val,
> + int retry_count);
> +};

We should also add the limit/usage write strategy function in here too.


> +
> struct res_counter {
> /*
> * the current resource consumption level
> @@ -39,6 +46,10 @@ struct res_counter {
> */
> unsigned long long failcnt;
> /*
> + * registered callbacks etc...for res_counter.
> + */
> + struct res_counter_ops ops;
> + /*

As Pavel mentioned, a pointer would be better here.
> -void res_counter_init(struct res_counter *counter);
> +void res_counter_init_ops(struct res_counter *counter,
> + struct res_counter_ops *ops);
> +
> +static inline void res_counter_init(struct res_counter *counter)
> +{
> + res_counter_init_ops(counter, NULL);
> +}

I would rather just see res_counter_init() take an ops parameter, and
update the (few) users of res_counter.


> +static int res_counter_resize_limit(struct res_counter *cnt,
> + unsigned long long val)
> +{
> + int retry_count = 0;
> + int ret = -EBUSY;
> + unsigned long flags;
> +
> + BUG_ON(!cnt->ops.shrink_usage);

As others have pointed out, there are some subsystems where usage
can't be shrunk. Maybe provide a "res_counter_unshrinkable()" function
that always returns -EBUSY and can be used by subsystems that can't
handle shrinking?

> @@ -133,11 +185,29 @@ ssize_t res_counter_write(struct res_cou
> if (*end != '\0')
> goto out_free;
> }
> + switch (member) {
> + case RES_LIMIT:
> + if (counter->ops.shrink_usage) {
> + ret = res_counter_resize_limit(counter, tmp);
> + goto done;
> + }
> + break;
> + default:
> + /*
> + * Considering future implementation, we'll have to handle
> + * other members and "fallback" will not work well. So, we
> + * avoid to make use of "default" here.
> + */
> + break;
> + }

Would this be simpler as just

if (member == RES_LIMIT && counter->ops.shrink_usage) {
ret = res_counter_resize_limit(counter, tmp);
} else {
...
}

Paul


\
 
 \ /
  Last update: 2008-06-20 07:13    [W:0.128 / U:1.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site