OpenVZ Forum - RDF feed
type: channel
items: [RFC 0/7] Initial proposal for faster res_counter updates
items: [RFC 2/7] consolidate all res_counter manipulation
items: [RFC 3/7] bundle a percpu counter into res_counters and use its lock
items: [RFC 4/7] move res_counter_set limit to res_counter.c
items: [RFC 5/7] use percpu_counters for res_counter usage
items: [RFC 6/7] Add min and max statistics to percpu_counter
items: [RFC 7/7] Global optimization
items: Re: [RFC 0/7] Initial proposal for faster res_counter updates
items: Re: [RFC 5/7] use percpu_counters for res_counter usage

[RFC 0/7] Initial proposal for faster res_counter updates
type: item
description: Hi, Here is my take about how we can make res_counter updates faster. Keep in mind this is a bit of a hack intended as a proof of concept. The pros I see with this: * free updates in non-constrained paths. non-constrained paths includes unlimited scenarios, but also ones in which we are far from the limit. * No need to have a special cache mechanism in memcg. The problem with the caching is my opinion, is that we will forward-account pages, meaning that we'll consider accounted pages we never used. I am not sure anyone actually ran into this, but in theory, this can fire events much earlier than it should. But the cons: * percpu counters have signed quantities, so this would limit us 4G. We can add a shift and then count pages instead of bytes, but we are still in the 16T area here. Maybe we really need more than that. * some of the additions here may slow down the percpu_counters for users that don't care about our usage. Things about min/max tracking enter in this category. * growth of the percpu memory. It is still not clear for me if we should use percpu_counters as this patch implies, or if we should just replicate its functionality. I need to go through at least one more full round of auditing before making sure the locking is safe, specially my use of synchronize_rcu(). As for measurements, the cache we have in memcg kind of distort things. I need to either disable it, or find the cases in which it is likely to lose and benchmark them, such as deep hierarchy concurrent updates with common parents. I also included a possible optimization that can be done when we are close to the limit to avoid the initial tests altogether, but it needs to be extended to avoid scanning the percpu areas as well. In summary, if this is to be carried forward, it definitely needs some love. It should be, however, more than enough to make the proposal clear. Comments are appreciated. Glauber Costa (7): split percpu_counter_sum consolidate all res_counter manipulation bundle a percpu…
creator: Glauber Costa
date: 2012-03-30T08:04:38-00:00

[RFC 2/7] consolidate all res_counter manipulation
type: item
description: This patch moves all the locked updates done to res_counter to __res_counter_add(). It gets flags for the special cases like nofail(), and a negative value of the increment means uncharge. This will be useful later when we start doing percpu_counter updates. Signed-off-by: Glauber Costa <glommer@parallels.com> --- kernel/res_counter.c | 59 +++++++++++++++++++++++-------------------------- 1 files changed, 28 insertions(+), 31 deletions(-) diff --git a/kernel/res_counter.c b/kernel/res_counter.c index b8a3d6a..ecb4aad 100644 --- a/kernel/res_counter.c +++ b/kernel/res_counter.c @@ -22,17 +22,32 @@ void res_counter_init(struct res_counter *counter, struct res_counter *parent) counter->parent = parent; } -int res_counter_charge_locked(struct res_counter *counter, unsigned long val) +int __res_counter_add(struct res_counter *c, long val, bool fail) { - if (counter->usage + val > counter->limit) { - counter->failcnt++; - return -ENOMEM; + int ret = 0; + u64 usage; + + spin_lock(&c->lock); + + usage = c->usage; + + if (usage + val > c->limit) { + c->failcnt++; + ret = -ENOMEM; + if (fail) + goto out; } - counter->usage += val; - if (counter->usage > counter->max_usage) - counter->max_usage = counter->usage; - return 0; + usage += val; + + c->usage = usage; + if (usage > c->max_usage) + c->max_usage = usage; + +out: + spin_unlock(&c->lock); + return ret; + } int res_counter_charge(struct res_counter *counter, unsigned long val, @@ -45,9 +60,7 @@ int res_counter_charge(struct res_counter *counter, unsigned long val, *limit_fail_at = NULL; local_irq_save(flags); for (c = counter; c != NULL; c = c->parent) { - spin_lock(&c->lock); - ret = res_counter_charge_locked(c, val); - spin_unlock(&c->lock); + ret = __res_counter_add(c, val, true); if (ret < 0) { *limit_fail_at = c; goto undo; @@ -57,9 +70,7 @@ int res_counter_charge(struct res_counter *counter, unsigned long val, goto done; undo: for (u = counter; u != c; u = u->parent) { - spin_lock(&u->lock); - res_counter_uncharge_l…
creator: Glauber Costa
date: 2012-03-30T08:04:40-00:00

[RFC 3/7] bundle a percpu counter into res_counters and use its lock
type: item
description: This is a preparation patch. It bundles a percpu_counter into the resource counter. But it doesn't do accounting with it just yet. Instead. this preparation patch removes the res_counter spinlock, and rely on the percpu_counter own lock for that. Over time, this need to be done with acessors if we really plan to merge it. But right now it can be used to give an idea about how it might be. Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/res_counter.h | 30 +++++++++++++----------------- kernel/res_counter.c | 15 ++++++++------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index a860183..d4f3674 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -26,6 +26,7 @@ struct res_counter { * the current resource consumption level */ unsigned long long usage; + struct percpu_counter usage_pcp; /* * the maximal value of the usage from the counter creation */ @@ -43,11 +44,6 @@ struct res_counter { */ unsigned long long failcnt; /* - * the lock to protect all of the above. - * the routines below consider this to be IRQ-safe - */ - spinlock_t lock; - /* * Parent counter, used for hierarchial resource accounting */ struct res_counter *parent; @@ -143,12 +139,12 @@ static inline unsigned long long res_counter_margin(struct res_counter *cnt) unsigned long long margin; unsigned long flags; - spin_lock_irqsave(&cnt->lock, flags); + raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); if (cnt->limit > cnt->usage) margin = cnt->limit - cnt->usage; else margin = 0; - spin_unlock_irqrestore(&cnt->lock, flags); + raw_spin_unlock_irqrestore(&cnt->usage_pcp.lock, flags); return margin; } @@ -165,12 +161,12 @@ res_counter_soft_limit_excess(struct res_counter *cnt) unsigned long long excess; unsigned long flags; - spin_lock_irqsave(&cnt->lock, flags); + raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); if (cnt->usage <= cnt->soft_limit) excess = 0; else excess = cnt->us…
creator: Glauber Costa
date: 2012-03-30T08:04:41-00:00

[RFC 4/7] move res_counter_set limit to res_counter.c
type: item
description: Preparation patch. Function is about to get complication to be inline. Move it to the main file for consistency. Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/res_counter.h | 17 ++--------------- kernel/res_counter.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index d4f3674..53b271c 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -188,21 +188,8 @@ static inline void res_counter_reset_failcnt(struct res_counter *cnt) raw_spin_unlock_irqrestore(&cnt->usage_pcp.lock, flags); } -static inline int res_counter_set_limit(struct res_counter *cnt, - unsigned long long limit) -{ - unsigned long flags; - int ret = -EBUSY; - - raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); - if (cnt->usage <= limit) { - cnt->limit = limit; - ret = 0; - } - raw_spin_unlock_irqrestore(&cnt->usage_pcp.lock, flags); - return ret; -} - +int res_counter_set_limit(struct res_counter *cnt, + unsigned long long limit); static inline int res_counter_set_soft_limit(struct res_counter *cnt, unsigned long long soft_limit) diff --git a/kernel/res_counter.c b/kernel/res_counter.c index 70c46c9..052efaf 100644 --- a/kernel/res_counter.c +++ b/kernel/res_counter.c @@ -111,6 +111,20 @@ void res_counter_uncharge(struct res_counter *counter, unsigned long val) local_irq_restore(flags); } +int res_counter_set_limit(struct res_counter *cnt, + unsigned long long limit) +{ + unsigned long flags; + int ret = -EBUSY; + + raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); + if (cnt->usage <= limit) { + cnt->limit = limit; + ret = 0; + } + raw_spin_unlock_irqrestore(&cnt->usage_pcp.lock, flags); + return ret; +} static inline unsigned long long * res_counter_member(struct res_counter *counter, int member) -- 1.7.4.1
creator: Glauber Costa
date: 2012-03-30T08:04:42-00:00

[RFC 5/7] use percpu_counters for res_counter usage
type: item
description: This is the bulk of the proposal. Updates to the res_counter are done to the percpu area, if we are inside what we can call the "safe zone". The safe zone is whenever we are far enough from the limit to be sure this update won't touch it. It is bigger the bigger the system is, since it grows with the number of cpus. However, for unlimited scenarios, this will always be the case. In those situations we are sure to never be close to the limit simply because the limit is high enough. Small consumers will also be safe. This includes workloads that pin and unpin memory often, but never grow the total size of memory by too much. The memory reported (reads of RES_USAGE) in this way is actually more precise than we currently have (Actually would be, if we would disable the memcg caches): I am using percpu_counter_sum(), meaning the cpu areas will be scanned and accumulated. percpu_counter_read() can also be used for reading RES_USAGE. We could then be off by a factor of batch_size * #cpus. I consider this to be not worse than the current situation with the memcg caches. Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/res_counter.h | 15 ++++++---- kernel/res_counter.c | 61 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index 53b271c..8c1c20e 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -25,7 +25,6 @@ struct res_counter { /* * the current resource consumption level */ - unsigned long long usage; struct percpu_counter usage_pcp; /* * the maximal value of the usage from the counter creation @@ -138,10 +137,12 @@ static inline unsigned long long res_counter_margin(struct res_counter *cnt) { unsigned long long margin; unsigned long flags; + u64 usage; raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); - if (cnt->limit > cnt->usage) - margin = cnt->limit - cnt->usage; + usage = __percpu_counter_sum_locked(&c…
creator: Glauber Costa
date: 2012-03-30T08:04:43-00:00

[RFC 6/7] Add min and max statistics to percpu_counter
type: item
description: Because percpu counters can accumulate their per-cpu sums over time outside of the control of the callers, we need this patch that updates the maximum values when that happens. I am adding a mininum value as well for conistency, due to the signed nature of the percpu_counters. However, I am not sure this will be of general use, and might be yet another indication that we need to duplicate those structures... Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/percpu_counter.h | 2 ++ include/linux/res_counter.h | 6 +----- kernel/res_counter.c | 6 +++--- lib/percpu_counter.c | 4 ++++ 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/linux/percpu_counter.h b/include/linux/percpu_counter.h index 8310548..639d2d5 100644 --- a/include/linux/percpu_counter.h +++ b/include/linux/percpu_counter.h @@ -18,6 +18,8 @@ struct percpu_counter { raw_spinlock_t lock; s64 count; + s64 max; + s64 min; #ifdef CONFIG_HOTPLUG_CPU struct list_head list; /* All percpu_counters are on a list */ #endif diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index 8c1c20e..3527827 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -27,10 +27,6 @@ struct res_counter { */ struct percpu_counter usage_pcp; /* - * 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; @@ -178,7 +174,7 @@ static inline void res_counter_reset_max(struct res_counter *cnt) unsigned long flags; raw_spin_lock_irqsave(&cnt->usage_pcp.lock, flags); - cnt->max_usage = __percpu_counter_sum_locked(&cnt->usage_pcp); + cnt->usage_pcp.max = __percpu_counter_sum_locked(&cnt->usage_pcp); raw_spin_unlock_irqrestore(&cnt->usage_pcp.lock, flags); } diff --git a/kernel/res_counter.c b/kernel/res_counter.c index 8a99943..7b05208 100644 --- a/kernel/res_counter.c +++ b/kernel/res_counter.c @@ -61,8 +61,8 @@ int __res_counter_add(struct res_counter …
creator: Glauber Costa
date: 2012-03-30T08:04:44-00:00

[RFC 7/7] Global optimization
type: item
description: When we are close to the limit, doing percpu_counter_add and its equivalent tests is a waste of time. This patch introduce a "global" state flag to the res_counter. When we are close to the limit, this flag is set and we skip directly to the locked part. The flag is unset when we are far enough away from the limit. In this mode, we function very much like the original resource counter The main difference right now is that we still scan all the cpus. This should however be very easy to avoid, with a flusher function that empties the per-cpu areas, and then updating usage_pcp directly. This should be doable because once we get the global flag, we know no one else would be adding to the percpu areas any longer. Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/res_counter.h | 1 + kernel/res_counter.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index 3527827..a8e4646 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -30,6 +30,7 @@ struct res_counter { * the limit that usage cannot exceed */ unsigned long long limit; + bool global; /* * the limit that usage can be exceed */ diff --git a/kernel/res_counter.c b/kernel/res_counter.c index 7b05208..859a27d 100644 --- a/kernel/res_counter.c +++ b/kernel/res_counter.c @@ -29,6 +29,8 @@ int __res_counter_add(struct res_counter *c, long val, bool fail) u64 usage; rcu_read_lock(); + if (c->global) + goto global; if (val < 0) { percpu_counter_add(&c->usage_pcp, val); @@ -45,9 +47,25 @@ int __res_counter_add(struct res_counter *c, long val, bool fail) return 0; } +global: rcu_read_unlock(); raw_spin_lock(&c->usage_pcp.lock); + usage = __percpu_counter_sum_locked(&c->usage_pcp); + + /* everyone that could update global is under lock + * reader could miss a transition, but that is not a problem, + * since we are always using percpu_counter_sum anyway + */ + + if (!c->global && val > 0 &…
creator: Glauber Costa
date: 2012-03-30T08:04:45-00:00

Re: [RFC 0/7] Initial proposal for faster res_counter updates
type: item
description: (2012/03/30 17:04), Glauber Costa wrote: > Hi, > > Here is my take about how we can make res_counter updates faster. > Keep in mind this is a bit of a hack intended as a proof of concept. > > The pros I see with this: > > * free updates in non-constrained paths. non-constrained paths includes > unlimited scenarios, but also ones in which we are far from the limit. > > * No need to have a special cache mechanism in memcg. The problem with > the caching is my opinion, is that we will forward-account pages, meaning > that we'll consider accounted pages we never used. I am not sure > anyone actually ran into this, but in theory, this can fire events > much earlier than it should. > Note: Assume a big system which has many cpus, and user wants to devide the system into containers. Current memcg's percpu caching is done only when a task in memcg is on the cpu, running. So, it's not so dangerous as it looks. But yes, if we can drop memcg's code, it's good. Then, we can remove some amount of codes. > But the cons: > > * percpu counters have signed quantities, so this would limit us 4G. > We can add a shift and then count pages instead of bytes, but we > are still in the 16T area here. Maybe we really need more than that. > .... struct percpu_counter { raw_spinlock_t lock; s64 count; s64 limtes us 4G ? > * some of the additions here may slow down the percpu_counters for > users that don't care about our usage. Things about min/max tracking > enter in this category. > I think it's not very good to increase size of percpu counter. It's already very big...Hm. How about struct percpu_counter_lazy { struct percpu_counter pcp; extra information s64 margin; } ? > * growth of the percpu memory. > This may be a concern. I'll look into patches. Thanks, -Kame > It is still not clear for me if we should use percpu_counters as this > patch implies, or if we should just replicate its functionality. > > I need to go through at least one more full round of auditing before > making sure the …
creator: KAMEZAWA Hiroyuki
date: 2012-03-30T08:32:06-00:00

Re: [RFC 5/7] use percpu_counters for res_counter usage
type: item
description: (2012/03/30 17:04), Glauber Costa wrote: > This is the bulk of the proposal. > Updates to the res_counter are done to the percpu area, if we are > inside what we can call the "safe zone". > > The safe zone is whenever we are far enough from the limit to be > sure this update won't touch it. It is bigger the bigger the system > is, since it grows with the number of cpus. > > However, for unlimited scenarios, this will always be the case. > In those situations we are sure to never be close to the limit simply > because the limit is high enough. > > Small consumers will also be safe. This includes workloads that > pin and unpin memory often, but never grow the total size of memory > by too much. > > The memory reported (reads of RES_USAGE) in this way is actually > more precise than we currently have (Actually would be, if we > would disable the memcg caches): I am using percpu_counter_sum(), > meaning the cpu areas will be scanned and accumulated. > > percpu_counter_read() can also be used for reading RES_USAGE. > We could then be off by a factor of batch_size * #cpus. I consider > this to be not worse than the current situation with the memcg caches. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > --- > include/linux/res_counter.h | 15 ++++++---- > kernel/res_counter.c | 61 ++++++++++++++++++++++++++++++++++++------- > 2 files changed, 60 insertions(+), 16 deletions(-) > > diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h > index 53b271c..8c1c20e 100644 > --- a/include/linux/res_counter.h > +++ b/include/linux/res_counter.h > @@ -25,7 +25,6 @@ struct res_counter { > /* > * the current resource consumption level > */ > - unsigned long long usage; > struct percpu_counter usage_pcp; > /* > * the maximal value of the usage from the counter creation > @@ -138,10 +137,12 @@ static inline unsigned long long res_counter_margin(struct res_counter *cnt) > { > unsigned long long margin; > unsigned long flags; > + u64 usage; > > raw_spin_lock_irqs…
creator: KAMEZAWA Hiroyuki
date: 2012-03-30T09:33:31-00:00

Re: [RFC 5/7] use percpu_counters for res_counter usage
type: item
description: (2012/03/30 18:33), KAMEZAWA Hiroyuki wrote: > (2012/03/30 17:04), Glauber Costa wrote: > > Hmm.... this part doesn't seem very good. > I don't think for_each_online_cpu() here will not be a way to the final win. > Under multiple hierarchy, you may need to call for_each_online_cpu() in each level. > > Can't you update percpu counter's core logic to avoid using for_each_online_cpu() ? > For example, if you know what cpus have caches, you can use that cpu mask... > > Memo: > Current implementation of memcg's percpu counting is reserving usage before its real use. > In usual, the kernel don't have to scan percpu caches and just drain caches from cpus > reserving usages if we need to cancel reserved usages. (And it's automatically canceled > when cpu's memcg changes.) > > And 'reserving' avoids caching in multi-level counters,....it updates multiple counters > in batch and memcg core don't need to walk res_counter ancestors in fast path. > > Considering res_counter's characteristics > - it has _hard_ limit > - it can be tree and usages are propagated to ancestors > - all ancestors has hard limit. > > Isn't it better to generalize 'reserving resource' model ? > You can provide 'precise usage' to the user by some logic. > Ah....one more point. please see this memcg's code. == if (nr_pages == 1 && consume_stock(memcg)) { /* * It seems dagerous to access memcg without css_get(). * But considering how consume_stok works, it's not * necessary. If consume_stock success, some charges * from this memcg are cached on this cpu. So, we * don't need to call css_get()/css_tryget() before * calling consume_stock(). */ rcu_read_unlock(); goto done; } /* after here, we may be blocked. we need to get refcnt */ if (!css_tryget(&memcg->css)) { rcu_read_unlock(); goto again; } == Now, we do consume 'reserved' usage, we can avoid css_get(), an heavy atomic ops. You may need to move this code as rcu_read_lock() .... res_counter_charge() if (failure) { css_tryget() rcu_read_unlock() } else { r…
creator: KAMEZAWA Hiroyuki
date: 2012-03-30T09:58:03-00:00

Re: [RFC 0/7] Initial proposal for faster res_counter updates
type: item
description: > Note: Assume a big system which has many cpus, and user wants to devide > the system into containers. Current memcg's percpu caching is done > only when a task in memcg is on the cpu, running. So, it's not so dangerous > as it looks. Agree. I actually think it is pretty > But yes, if we can drop memcg's code, it's good. Then, we can remove some > amount of codes. > >> But the cons: >> >> * percpu counters have signed quantities, so this would limit us 4G. >> We can add a shift and then count pages instead of bytes, but we >> are still in the 16T area here. Maybe we really need more than that. >> > > .... > struct percpu_counter { > raw_spinlock_t lock; > s64 count; > > s64 limtes us 4G ? > Yes, I actually explicitly mentioned that. We can go to 16T if we track pages instead of bytes (I considered having the res_counter initialization code to specify a shift, so we could be generic). But I believe that if we go this route, we'll need to either: 1) Have our own internal implementation of what percpu counters does 2) create u64 acessors that would cast that to u64 in the operations. Since it is a 64 bit field anyway it should be doable. But being doable doesn't mean we should do it.... 3) Have a different percpu_counter structure, something like struct percpu_positive_counter. > >> * some of the additions here may slow down the percpu_counters for >> users that don't care about our usage. Things about min/max tracking >> enter in this category. >> > > > I think it's not very good to increase size of percpu counter. It's already > very big...Hm. How about > > struct percpu_counter_lazy { > struct percpu_counter pcp; > extra information > s64 margin; > } > ? Can work, but we need something that also solves the signedness problem. Maybe we can use a union for that, and then stuff things in the end of a different structure just for the users that want it.
creator: Glauber Costa
date: 2012-03-30T10:46:17-00:00

Re: [RFC 5/7] use percpu_counters for res_counter usage
type: item
description: > diff --git a/kernel/res_counter.c b/kernel/res_counter.c >> index 052efaf..8a99943 100644 >> --- a/kernel/res_counter.c >> +++ b/kernel/res_counter.c >> @@ -28,9 +28,28 @@ int __res_counter_add(struct res_counter *c, long val, bool fail) >> int ret = 0; >> u64 usage; >> >> + rcu_read_lock(); >> + > > > Hmm... isn't it better to synchronize percpu usage to the main counter > by smp_call_function() or some at set limit ? after set 'global' mode ? Yes. I think it should be done after global mode is set. My idea is to flush all the percpu data, and then start treating that essentially as a res_counter is today. > > set global mode > smp_call_function(drain all pcp counters to main counter) > set limit. > unset global mode > >> + if (val< 0) { >> + percpu_counter_add(&c->usage_pcp, val); >> + rcu_read_unlock(); >> + return 0; >> + } > > > Memo: > memcg's uncharge path is batched ....so..it will be bigger than > percpu_counter_batch() in most of cases. (And lock conflict is enough low.) > I don't get what you mean. It is batched, because charge is batched. If we un-batch one, we un-batch another. And if we don't we don't do for any. >> + >> + usage = percpu_counter_read(&c->usage_pcp); >> + >> + if (percpu_counter_read(&c->usage_pcp) + val< >> + (c->limit + num_online_cpus() * percpu_counter_batch)) { > > > c->limit - num_online_cpus() * percpu_counter_batch ? > > Anyway, you can pre-calculate this value at cpu hotplug event.. Beautiful. Haven't thought about that. Thanks. > >> + percpu_counter_add(&c->usage_pcp, val); >> + rcu_read_unlock(); >> + return 0; >> + } >> + >> + rcu_read_unlock(); >> + >> raw_spin_lock(&c->usage_pcp.lock); >> >> - usage = c->usage; >> + usage = __percpu_counter_sum_locked(&c->usage_pcp); > > > Hmm.... this part doesn't seem very good. > I don't think for_each_online_cpu() here will not be a way to the final win. > Under multiple hierarchy, you may need to call for_each_online_cpu() in each level. > > Can't you update percpu counter's core lo…
creator: Glauber Costa
date: 2012-03-30T12:59:32-00:00

Re: [RFC 5/7] use percpu_counters for res_counter usage
type: item
description: On 03/30/2012 11:58 AM, KAMEZAWA Hiroyuki wrote: > == > > Now, we do consume 'reserved' usage, we can avoid css_get(), an heavy atomic > ops. You may need to move this code as > > rcu_read_lock() > .... > res_counter_charge() > if (failure) { > css_tryget() > rcu_read_unlock() > } else { > rcu_read_unlock() > return success; > } > > to compare performance. This css_get() affects performance very very much. thanks for the tip. But one thing: To be sure: it effectively mean that we are drawing from a dead memcg (because we pre-allocated, right?
creator: Glauber Costa
date: 2012-03-30T13:53:16-00:00

Re: [RFC 5/7] use percpu_counters for res_counter usage
type: item
description: (2012/03/30 22:53), Glauber Costa wrote: > On 03/30/2012 11:58 AM, KAMEZAWA Hiroyuki wrote: >> == >> >> Now, we do consume 'reserved' usage, we can avoid css_get(), an heavy atomic >> ops. You may need to move this code as >> >> rcu_read_lock() >> .... >> res_counter_charge() >> if (failure) { >> css_tryget() >> rcu_read_unlock() >> } else { >> rcu_read_unlock() >> return success; >> } >> >> to compare performance. This css_get() affects performance very very much. > > thanks for the tip. > > But one thing: > > To be sure: it effectively mean that we are drawing from a dead memcg > (because we pre-allocated, right? Cached stock is consumed by the current task. It blocks removal of memcg. It's not dead. Thanks, -Kame
creator: KAMEZAWA Hiroyuki
date: 2012-04-09T01:48:42-00:00