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

Paul Menage
2008-10-21T01:29:31Z

On Mon, Oct 20, 2008 at 6:14 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> 1. It's harmful to increase size of *generic* res_counter. So, modifing
> res_counter only for us is not a choice.

Adding an extra pointer to a per-cgroup structure isn't particularly harmful.

> 2. Operation should be done under a lock. We have to do
> -page + swap in atomic, at least.

How bad would things really be if you did something like the code below?

if (charge_swap()) {
uncharge_mem();
} else {
return -ENOMEM;
}

It's true that this introduces a tiny race whereby a single swap-in
page allocation that might have succeeded could fail, but if you're
that close to the limit your cgroup is heading for an OOM anyway.

> 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?

>
>> 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.

Paul