From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753825AbaDRQHc (ORCPT ); Fri, 18 Apr 2014 12:07:32 -0400 Received: from relay.parallels.com ([195.214.232.42]:55777 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394AbaDRQH2 (ORCPT ); Fri, 18 Apr 2014 12:07:28 -0400 Message-ID: <53514DBF.3040508@parallels.com> Date: Fri, 18 Apr 2014 20:07:27 +0400 From: Vladimir Davydov MIME-Version: 1.0 To: Johannes Weiner CC: , , , , , , , Subject: Re: [PATCH RFC -mm v2 2/3] memcg, slab: merge memcg_{bind,release}_pages to memcg_{un}charge_slab References: <49f7f2d048e56fac4d29dd5b39f6f76c7bdd6bec.1397804745.git.vdavydov@parallels.com> <20140418134453.GC26283@cmpxchg.org> In-Reply-To: <20140418134453.GC26283@cmpxchg.org> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.30.16.96] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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.