On 06/04/2015 11:56 AM, Vladimir Davydov wrote:
> On Wed, Jun 03, 2015 at 04:34:21PM +0300, Pavel Emelyanov wrote:
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 9dda309..d38868c 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -498,7 +498,6 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup
>> *memcg)
>>
>> /* Writing them here to avoid exposing memcg's inner layout */
>> #if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
>> -
>> void sock_update_memcg(struct sock *sk)
>> {
>> if (mem_cgroup_sockets_enabled) {
>> @@ -3039,11 +3038,33 @@ int memcg_charge_kmem(struct mem_cgroup *memcg,
>> gfp_t gfp, u64 size)
>> return ret;
>> }
>>
>> -void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
>
> I've changed this code recently, so this patch does not apply. Please
> update.
:( If this is just context change, then Kostja can just manually fix it. Please.
>> +void memcg_charge_kmem_nofail(struct mem_cgroup *memcg, u64 size)
>> {
>> + struct res_counter *fail_res;
>> +
>> + /*
>> + * FIXME -- strictly speaking, this value should _also_
>> + * be charged into kmem counter. But since res_counter_charge
>> + * is sub-optimal (takes locks) AND we do not care much
>> + * about kmem limits (at least for now) we can just directly
>> + * charge into mem counter.
>> + */
>
> Please charge kmem too. As I've already told you it should not make any
> difference in terms of performance, because we already have a bottleneck
> of the same bandwidth.
>
> Anyway, if we see any performance degradation, I will convert
> mem_cgroup->kmem to a percpu counter.
No, let's do it vice-versa -- first you fix the locking, then I update this
code.
>> + res_counter_charge_nofail(&memcg->res, size, &fail_res);
>> + if (do_swap_account)
>> + res_counter_uncharge(&memcg->memsw, size);
>> +}
>> +
>> +void __memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
>> +{
>> + /* FIXME -- uncharge also in kmem counter */
>
> Please use memcg_uncharge_kmem instead of introducing this new function
> with its ridiculous divergence between its name and actual doings.
The comment in "charge" routine explains this divergence. I'm not quite happy
with the fact that kmem_charge will do 3 (three) locking-in-a-loop. Please,
fix this and I'll update the networking acct code.
>> res_counter_uncharge(&memcg->res, size);
>> if (do_swap_account)
>> res_counter_uncharge(&memcg->memsw, size);
>> +}
>> +
>> +void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
>> +{
>> + __memcg_uncharge_kmem(memcg, size);
>>
>> /* Not down to 0 */
>> if (res_counter_uncharge(&memcg->kmem, size))
> .
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
