On 06/04/2015 01:46 PM, Vladimir Davydov wrote:
> On Thu, Jun 04, 2015 at 01:07:08PM +0300, Pavel Emelyanov wrote:
>>>>>> +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.
>>>
>>> I don't understand why, because you provide no arguments and keep
>>> ignoring my reasoning why I think charging kmem along with res is OK,
>>> which is one paragraph above.
>>
>> The bandwidth of the bottleneck doesn't look to be the same -- the res
>> counters
>> in question are not in one cache-line and adding one more
>
>> (btw, do we have swap account turned on by default?)
>
> Yes it is. We'll have to switch to percpu stocks here.
>
>> will not come unnoticed.
>
> Fair enough. But there are already 3 calls to res_counter_charge in a
:(
> row, which is terrible and must be reworked. My point is that this vague
> performance implications are not worth complicating the code that badly
> w/o any hope to recover performance back to the level w/o using cgroups.
> Performance must be up to a separate task.
The code complication is not enormous. I will have to introduce the
memcg_charge_kmem_nofail
anyway, so we're arguing here simply whether or not to use my version of
__memcg_uncharge_kmem
or existing version of memcg_uncharge_kmem.
>>
>> Yet again -- I don't mind changing this and charge TCP into kmem too, I'll do
>> it, but after this charging becomes fast enough.
>
> Once again, performance improvements are not for Beta1. I'll file a
> separate task for Beta2 for switching from atomics/spinlocks to percpu
> counters wherever possible.
Ah, not even for Beta1. OK, I'll tune that up.
-- Pavel
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
