> On Aug 29, 2024, at 23:49, Shakeel Butt <shakeel.butt@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> 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
