OpenVZ Forum


Home » Mailing lists » Devel » Re: [PATCH 07/10] memcg: Stop res_counter underflows.
Re: [PATCH 07/10] memcg: Stop res_counter underflows. [message #45383] Wed, 29 February 2012 17:05 Go to next message
Glauber Costa is currently offline  Glauber Costa
Messages: 916
Registered: October 2011
Senior Member
From: *virtua.com.br
On 02/28/2012 08:07 PM, Suleiman Souhlal wrote:
> On Tue, Feb 28, 2012 at 5:31 AM, Glauber Costa<glommer@parallels.com> wrote:
>> I don't fully understand this.
>> To me, the whole purpose of having a cache tied to a memcg, is that we know
>> all allocations from that particular cache should be billed to a specific
>> memcg. So after a cache is created, and has an assigned memcg,
>> what's the point in bypassing it to root?
>>
>> It smells like you're just using this to circumvent something...
>
> In the vast majority of the cases, we will be able to account to the cgroup.
> However, there are cases when __mem_cgroup_try_charge() is not able to
> do so, like when the task is being killed.
> When this happens, the allocation will not get accounted to the
> cgroup, but the slab accounting code will still think the page belongs
> to the memcg's kmem_cache.
> So, when we go to free the page, we assume that the page belongs to
> the memcg and uncharge it, even though it was never charged to us in
> the first place.
>
> This is the situation this patch is trying to address, by keeping a
> counter of how much memory has been bypassed like this, and uncharging
> from the root if we have any outstanding bypassed memory.
>
> Does that make sense?
>
Yes, but how about the following:

I had a similar problem in tcp accounting, and solved that by adding
res_counter_charge_nofail().

I actually implemented something very similar to your bypass (now that I
understand it better...) and gave up in favor of this.

The tcp code has its particularities, but still, that could work okay
for the general slab.

Reason being:

Consider you have a limit of X, and is currently at X-1. You bypassed a
page.

So in reality, you should fail the next allocation, but you will not -
(unless you start considering the bypassed memory at allocation time as
well).

If you use res_counter_charge_nofail(), you will:

1) Still proceed with the allocations that shouldn't fail - so no
difference here
2) fail the normal allocations if you have "bypassed" memory filling
up your limit
3) all that without coupling something alien to the res_counter API.
Re: [PATCH 07/10] memcg: Stop res_counter underflows. [message #45803 is a reply to message #45383] Wed, 29 February 2012 19:17 Go to previous message
Suleiman Souhlal is currently offline  Suleiman Souhlal
Messages: 64
Registered: February 2012
Member
From: *parallels.com
On Wed, Feb 29, 2012 at 9:05 AM, Glauber Costa <glommer@parallels.com> wrote:
> On 02/28/2012 08:07 PM, Suleiman Souhlal wrote:
>>
>> On Tue, Feb 28, 2012 at 5:31 AM, Glauber Costa<glommer@parallels.com>
>>  wrote:
>>>
>>> I don't fully understand this.
>>> To me, the whole purpose of having a cache tied to a memcg, is that we
>>> know
>>> all allocations from that particular cache should be billed to a specific
>>> memcg. So after a cache is created, and has an assigned memcg,
>>> what's the point in bypassing it to root?
>>>
>>> It smells like you're just using this to circumvent something...
>>
>>
>> In the vast majority of the cases, we will be able to account to the
>> cgroup.
>> However, there are cases when __mem_cgroup_try_charge() is not able to
>> do so, like when the task is being killed.
>> When this happens, the allocation will not get accounted to the
>> cgroup, but the slab accounting code will still think the page belongs
>> to the memcg's kmem_cache.
>> So, when we go to free the page, we assume that the page belongs to
>> the memcg and uncharge it, even though it was never charged to us in
>> the first place.
>>
>> This is the situation this patch is trying to address, by keeping a
>> counter of how much memory has been bypassed like this, and uncharging
>> from the root if we have any outstanding bypassed memory.
>>
>> Does that make sense?
>>
> Yes, but how about the following:
>
> I had a similar problem in tcp accounting, and solved that by adding
> res_counter_charge_nofail().
>
> I actually implemented something very similar to your bypass (now that I
> understand it better...) and gave up in favor of this.
>
> The tcp code has its particularities, but still, that could work okay for
> the general slab.
>
> Reason being:
>
> Consider you have a limit of X, and is currently at X-1. You bypassed a
> page.
>
> So in reality, you should fail the next allocation, but you will not -
> (unless you start considering the bypassed memory at allocation time as
> well).
>
> If you use res_counter_charge_nofail(), you will:
>
>  1) Still proceed with the allocations that shouldn't fail - so no
>    difference here
>  2) fail the normal allocations if you have "bypassed" memory filling
>    up your limit
>  3) all that without coupling something alien to the res_counter API.

Ok. I'll give it a try.

Thanks!
-- Suleiman
Previous Topic: Re: [PATCH 04/10] memcg: Introduce __GFP_NOACCOUNT.
Next Topic: Re: [PATCH 08/10] memcg: Add CONFIG_CGROUP_MEM_RES_CTLR_KMEM_ACCT_ROOT.
Goto Forum:
  


Current Time: Sat Apr 04 09:18:28 GMT 2020