[Devel] Re: [PATCH RFC v0 2/3] res_counter: implement thresholds
On Fri, Nov 27, 2009 at 5:08 AM, Balbir Singh bal...@linux.vnet.ibm.com wrote: On Fri, Nov 27, 2009 at 8:15 AM, KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com wrote: On Fri, 27 Nov 2009 09:20:35 +0900 Daisuke Nishimura nishim...@mxp.nes.nec.co.jp wrote: Hi. @@ -73,6 +76,7 @@ void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val) val = counter-usage; counter-usage -= val; + res_counter_threshold_notify_locked(counter); } hmm.. this adds new checks to hot-path of process life cycle. Do you have any number on performance impact of these patches(w/o setting any threshold)? No, I don't. I did only functional testing on this stage. IMHO, it might be small enough to be ignored because KAMEZAWA-san's coalesce charge/uncharge patches have decreased charge/uncharge for res_counter itself, but I want to know just to make sure. Another concern is to support root cgroup, you need another notifier hook in memcg because root cgroup doesn't use res_counter now. Can't this be implemented in a way like softlimit check ? I'll investigate it. Filter by the number of event will be good for notifier behavior, for avoiding too much wake up, too. Good idea, thanks. I guess the semantics would vary then, they would become activity semantics. I think we should avoid threshold notification for root, since we have no limits in root anymore. Threshold notifications for root cgroup is really needed on embedded systems to avid OOM-killer. BTW, Kirill, I've been meaning to write this layer on top of cgroupstats, is there anything that prevents us from using that today? I'll investigate it. CC'ing Dan Malek and Vladslav Buzov who worked on similar patches earlier. Balbir Singh. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH RFC v0 2/3] res_counter: implement thresholds
Hi. On Thu, 26 Nov 2009 19:11:16 +0200, Kirill A. Shutemov kir...@shutemov.name wrote: It allows to setup two thresholds: one above current usage and one below. Callback threshold_notifier() will be called if a threshold is crossed. Signed-off-by: Kirill A. Shutemov kir...@shutemov.name --- include/linux/res_counter.h | 44 +++ kernel/res_counter.c|4 +++ 2 files changed, 48 insertions(+), 0 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index fcb9884..bca99a5 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -9,6 +9,10 @@ * * Author: Pavel Emelianov xe...@openvz.org * + * Thresholds support + * Copyright (C) 2009 Nokia Corporation + * Author: Kirill A. Shutemov + * * See Documentation/cgroups/resource_counter.txt for more * info about what this counter is. */ @@ -42,6 +46,13 @@ struct res_counter { * the number of unsuccessful attempts to consume the resource */ unsigned long long failcnt; + + unsigned long long threshold_above; + unsigned long long threshold_below; + void (*threshold_notifier)(struct res_counter *counter, + unsigned long long usage, + unsigned long long threshold); + /* * the lock to protect all of the above. * the routines below consider this to be IRQ-safe @@ -145,6 +156,20 @@ static inline bool res_counter_soft_limit_check_locked(struct res_counter *cnt) return false; } +static inline void res_counter_threshold_notify_locked(struct res_counter *cnt) +{ + if (cnt-usage = cnt-threshold_above) { + cnt-threshold_notifier(cnt, cnt-usage, cnt-threshold_above); + return; + } + + if (cnt-usage cnt-threshold_below) { + cnt-threshold_notifier(cnt, cnt-usage, cnt-threshold_below); + return; + } +} + + /** * Get the difference between the usage and the soft limit * @cnt: The counter @@ -238,4 +263,23 @@ res_counter_set_soft_limit(struct res_counter *cnt, return 0; } +static inline int +res_counter_set_thresholds(struct res_counter *cnt, + unsigned long long threshold_above, + unsigned long long threshold_below) +{ + unsigned long flags; + int ret = -EINVAL; + + spin_lock_irqsave(cnt-lock, flags); + if ((cnt-usage threshold_above) + (cnt-usage = threshold_below)) { + cnt-threshold_above = threshold_above; + cnt-threshold_below = threshold_below; + ret = 0; + } + spin_unlock_irqrestore(cnt-lock, flags); + return ret; +} + #endif diff --git a/kernel/res_counter.c b/kernel/res_counter.c index bcdabf3..646c29c 100644 --- a/kernel/res_counter.c +++ b/kernel/res_counter.c @@ -20,6 +20,8 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent) spin_lock_init(counter-lock); counter-limit = RESOURCE_MAX; counter-soft_limit = RESOURCE_MAX; + counter-threshold_above = RESOURCE_MAX; + counter-threshold_below = 0ULL; counter-parent = parent; } @@ -33,6 +35,7 @@ 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; + res_counter_threshold_notify_locked(counter); return 0; } @@ -73,6 +76,7 @@ void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val) val = counter-usage; counter-usage -= val; + res_counter_threshold_notify_locked(counter); } hmm.. this adds new checks to hot-path of process life cycle. Do you have any number on performance impact of these patches(w/o setting any threshold)? IMHO, it might be small enough to be ignored because KAMEZAWA-san's coalesce charge/uncharge patches have decreased charge/uncharge for res_counter itself, but I want to know just to make sure. Regards, Daisuke Nishimura. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH RFC v0 2/3] res_counter: implement thresholds
On Fri, 27 Nov 2009 09:20:35 +0900 Daisuke Nishimura nishim...@mxp.nes.nec.co.jp wrote: Hi. @@ -73,6 +76,7 @@ void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val) val = counter-usage; counter-usage -= val; + res_counter_threshold_notify_locked(counter); } hmm.. this adds new checks to hot-path of process life cycle. Do you have any number on performance impact of these patches(w/o setting any threshold)? IMHO, it might be small enough to be ignored because KAMEZAWA-san's coalesce charge/uncharge patches have decreased charge/uncharge for res_counter itself, but I want to know just to make sure. Another concern is to support root cgroup, you need another notifier hook in memcg because root cgroup doesn't use res_counter now. Can't this be implemented in a way like softlimit check ? Filter by the number of event will be good for notifier behavior, for avoiding too much wake up, too. Thanks, -Kame ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH RFC v0 2/3] res_counter: implement thresholds
On Fri, Nov 27, 2009 at 8:15 AM, KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com wrote: On Fri, 27 Nov 2009 09:20:35 +0900 Daisuke Nishimura nishim...@mxp.nes.nec.co.jp wrote: Hi. @@ -73,6 +76,7 @@ void res_counter_uncharge_locked(struct res_counter *counter, unsigned long val) val = counter-usage; counter-usage -= val; + res_counter_threshold_notify_locked(counter); } hmm.. this adds new checks to hot-path of process life cycle. Do you have any number on performance impact of these patches(w/o setting any threshold)? IMHO, it might be small enough to be ignored because KAMEZAWA-san's coalesce charge/uncharge patches have decreased charge/uncharge for res_counter itself, but I want to know just to make sure. Another concern is to support root cgroup, you need another notifier hook in memcg because root cgroup doesn't use res_counter now. Can't this be implemented in a way like softlimit check ? Filter by the number of event will be good for notifier behavior, for avoiding too much wake up, too. I guess the semantics would vary then, they would become activity semantics. I think we should avoid threshold notification for root, since we have no limits in root anymore. BTW, Kirill, I've been meaning to write this layer on top of cgroupstats, is there anything that prevents us from using that today? CC'ing Dan Malek and Vladslav Buzov who worked on similar patches earlier. Balbir Singh. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel