Re: [PATCH RFC -mm v2 2/3] memcg, slab: merge memcg_{bind,release}_pages to memcg_{un}charge_slab

[Vladimir Davydov]

On 04/18/2014 05:44 PM, Johannes Weiner wrote:
> On Fri, Apr 18, 2014 at 12:04:48PM +0400, Vladimir Davydov wrote:
>> Currently we have two pairs of kmemcg-related functions that are called
>> on slab alloc/free. The first is memcg_{bind,release}_pages that count
>> the total number of pages allocated on a kmem cache. The second is
>> memcg_{un}charge_slab that {un}charge slab pages to kmemcg resource
>> counter. Let's just merge them to keep the code clean.
>>
>> Signed-off-by: Vladimir Davydov <vdavydov@xxxxxxxxxxxxx>
>> ---
>>   include/linux/memcontrol.h |    4 ++--
>>   mm/memcontrol.c            |   22 ++++++++++++++++++++--
>>   mm/slab.c                  |    2 --
>>   mm/slab.h                  |   25 ++-----------------------
>>   mm/slub.c                  |    2 --
>>   5 files changed, 24 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index 087a45314181..d38d190f4cec 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -506,8 +506,8 @@ void memcg_update_array_size(int num_groups);
>>   struct kmem_cache *
>>   __memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
>>
>> -int memcg_charge_kmem(struct mem_cgroup *memcg, gfp_t gfp, u64 size);
>> -void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size);
>> +int __memcg_charge_slab(struct kmem_cache *cachep, gfp_t gfp, int order);
>> +void __memcg_uncharge_slab(struct kmem_cache *cachep, int order);
>
> I like the patch overall, but why the __prefix and not just
> memcg_charge_slab() and memcg_uncharge_slab()?

Because I have memcg_{un}charge_slab (without underscores) in mm/slab.h.
Those functions are inline so that we only issue a function call if the
memcg_kmem_enabled static key is on and the cache is not a global one.

Actually I'm not sure if we really need such an optimization in slab
allocation/free paths, which are not very hot, but it wouldn't hurt,
would it?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/