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

KAMEZAWA Hiroyuki
2008-10-21T01:49:58Z

On Mon, 20 Oct 2008 18:29:28 -0700
"Paul Menage" <menage@google.com> wrote:
> How bad would things really be if you did something like the code below?
>
> if (charge_swap()) {
> uncharge_mem();
> } else {
> return -ENOMEM;
> }
Nitpick, charge_swap() has to always succeed (because mem+swap counter.
So,

BUG_ON(charge_swap());
or
charge_swap_and_uncharge_mem();

is necessary. Swap itself has no limit.

> > 3. We want to pack all member into a cache-line, multiple res_counter
> > is no good.
>
> As I said previously, if we do a prefetch on the aggregated
> res_counter before we touch any fields in the basic counter, then in
> theory we should never have to wait on a cache miss on the aggregated
> counter - either we have no misses (if both were in cache) or we fetch
> both lines concurrently (if neither were in cache). Do you think that
> reasoning is invalid?

res_counter's operation is very short.
take a lock => add and compare. => unlock.

So, I wonder there is not enough runway to do prefetch.
(Considering memcg, we can place 2 counters on a cacheline that by putting 2
counters on aligned line.)

> >
> >> Maybe have an "aggregate" pointer in a res_counter that points to
> >> another res_counter that sums some number of counters; both the mem
> >> and the swap res_counter objects for a cgroup would point to the
> >> mem+swap res_counter for their aggregate. Adjusting the usage of a
> >> counter would also adjust its aggregate (or fail if adjusting the
> >> aggregate failed).
> >>
> > It's complicated.
>
> Agreed, it's a bit more complicated than defining a new structure and
> code that's very reminiscent of res_counter. But it does solve the
> problem of aggregating across multiple resource types and multiple
> children in a generic way.
>
If you give me NACK, maybe I have to try that..
(But I believe aggregated child-parent counter will be verrrry slow.)

BTW, can we have *unsigned long* version of res_counter ?
memcg doesn't need *unsigned long long*.

And as another discussion, I'd like optimize res_counter by per_cpu.
This will be impossible with counters tied by pointer.

Thanks,
-Kame