Re: [PATCH -mm 1/5] memcg: replace res_counter

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

On Mon, 20 Oct 2008 19:20:16 -0700
"Paul Menage" <menage@google.com> wrote:

> On Mon, Oct 20, 2008 at 6:49 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > If you give me NACK, maybe I have to try that..
> > (But I believe aggregated child-parent counter will be verrrry slow.)
> 
> If it's really impossible to implement the aggregated version without
> a significant performance hit then that might be a reason to have a
> separate counter class. But I'd rather have a clean generic solution
> if we can manage it.
> 
I think we can't do without performance hit. Considering parent<->child counter,
parent is busier than child if usage is propergated from child to parent. So,
prefetch will be just a smal help.

> > BTW, can we have *unsigned long* version of res_counter ?
> > memcg doesn't need *unsigned long long*.
> 
> Potentially - but how often is a read-only operation on the
> performance sensitive path? Don't most fast-path operations that
> involve a res_counter have an update on the res_counter when they
> succeed? In which case you have to pull the cache line into a Modified
> state anyway.
>
I don't like *unsigned long long* just because we have to do following
=
   res->usage < *some number*
=
or
=
   val = res->usage.
=
always under lock because usage is unsigned long long.

> >
> > And as another discussion, I'd like optimize res_counter by per_cpu.
> 
> What were you thinking of doing for this?
> 
just an idea. I believe a process has locality to a res_counter.

==
  struct res_counter_cache {
	unsigned long cache;
	struct res_counter	*res;
  }
  DEFINE_PER_CPU(res_counter_cache, pcp_memcg_res);

  res_counter_charge(struct res_counter *res, struct res_counter_cache *cache, num)
  {
	if (cache->res == res && cache->res >= num) {
		cache->cache -= num;
		return 0;
	} else if (cache->res != res) {
		/* forget cache */
		spin_lock(cache->res);
		cache->res->usage -= cache->cache;
		cache->cache = NULL;
		spin_unlock(cache->res);
  	}
        ....
	cache->cache = res;
  }
  ....
==

But not have a fragile of code.

Thanks,
-Kame













--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>