[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <6088647D-147A-4704-BBA1-8CEDEDAE2885@linux.dev>
Date: Fri, 30 Aug 2024 14:09:57 +0800
From: Muchun Song <muchun.song@...ux.dev>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Roman Gushchin <roman.gushchin@...ux.dev>,
Andrew Morton <akpm@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>,
Michal Hocko <mhocko@...nel.org>,
Vlastimil Babka <vbabka@...e.cz>,
David Rientjes <rientjes@...gle.com>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
Eric Dumazet <edumazet@...gle.com>,
"David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Linux Memory Management List <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>,
Meta kernel team <kernel-team@...a.com>,
cgroups@...r.kernel.org,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v1] memcg: add charging of already allocated slab objects
> On Aug 29, 2024, at 23:49, Shakeel Butt <shakeel.butt@...ux.dev> wrote:
>
> On Thu, Aug 29, 2024 at 10:36:01AM GMT, Muchun Song wrote:
>>
>>
>>> On Aug 29, 2024, at 03:03, Shakeel Butt <shakeel.butt@...ux.dev> wrote:
>>>
>>> Hi Muchun,
>>>
>>> On Wed, Aug 28, 2024 at 10:36:06AM GMT, Muchun Song wrote:
>>>>
>>>>
>>>>> On Aug 28, 2024, at 01:23, Shakeel Butt <shakeel.butt@...ux.dev> wrote:
>>>>>
>>> [...]
>>>>>>
>>>>>> Does it handle the case of a too-big-to-be-a-slab-object allocation?
>>>>>> I think it's better to handle it properly. Also, why return false here?
>>>>>>
>>>>>
>>>>> Yes I will fix the too-big-to-be-a-slab-object allocations. I presume I
>>>>> should just follow the kfree() hanlding on !folio_test_slab() i.e. that
>>>>> the given object is the large or too-big-to-be-a-slab-object.
>>>>
>>>> Hi Shakeel,
>>>>
>>>> If we decide to do this, I suppose you will use memcg_kmem_charge_page
>>>> to charge big-object. To be consistent, I suggest renaming kmem_cache_charge
>>>> to memcg_kmem_charge to handle both slab object and big-object. And I saw
>>>> all the functions related to object charging is moved to memcontrol.c (e.g.
>>>> __memcg_slab_post_alloc_hook), so maybe we should also do this for
>>>> memcg_kmem_charge?
>>>>
>>>
>>> If I understand you correctly, you are suggesting to handle the general
>>> kmem charging and slab's large kmalloc (size > KMALLOC_MAX_CACHE_SIZE)
>>> together with memcg_kmem_charge(). However that is not possible due to
>>> slab path updating NR_SLAB_UNRECLAIMABLE_B stats while no updates for
>>> this stat in the general kmem charging path (__memcg_kmem_charge_page in
>>> page allocation code path).
>>>
>>> Also this general kmem charging path is used by many other users like
>>> vmalloc, kernel stack and thus we can not just plainly stuck updates to
>>> NR_SLAB_UNRECLAIMABLE_B in that path.
>>
>> Sorry, maybe I am not clear . To make sure we are on the same page, let
>> me clarify my thought. In your v2, I thought if we can rename
>> kmem_cache_charge() to memcg_kmem_charge() since kmem_cache_charge()
>> already has handled both big-slab-object (size > KMALLOC_MAX_CACHE_SIZE)
>> and small-slab-object cases. You know, we have a function of
>> memcg_kmem_charge_page() which could be used for charging big-slab-object
>> but not small-slab-object. So I thought maybe memcg_kmem_charge() is a
>> good name for it to handle both cases. And if we do this, how about moving
>> this new function to memcontrol.c since all memcg charging functions are
>> moved to memcontrol.c instead of slub.c.
>>
>
> Oh you want the core function to be in memcontrol.c. I don't have any
> strong opinion where the code should exist but I do want the interface
> to still be kmem_cache_charge() because that is what we are providing to
> the users which charging slab objects. Yes some of those might be
> big-slab-objects but that is transparent to the users.
>
> Anyways, for now I will go with my current approach but on the followup
> will explore and discuss with you on which code should exist in which
> file. I hope that is acceptable to you.
Fine. No problem.
Thanks.
>
> thanks,
> Shakeel
Powered by blists - more mailing lists