[Devel] Re: [PATCH RFC v0 2/3] res_counter: implement thresholds

2009-12-07 Thread Kirill A. Shutemov
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

2009-11-26 Thread Daisuke Nishimura
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

2009-11-26 Thread KAMEZAWA Hiroyuki
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

2009-11-26 Thread Balbir Singh
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