Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754327AbaBDO72 (ORCPT ); Tue, 4 Feb 2014 09:59:28 -0500 Received: from relay.parallels.com ([195.214.232.42]:40188 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754050AbaBDO70 (ORCPT ); Tue, 4 Feb 2014 09:59:26 -0500 Message-ID: <52F1004B.90307@parallels.com> Date: Tue, 4 Feb 2014 18:59:23 +0400 From: Vladimir Davydov MIME-Version: 1.0 To: Michal Hocko CC: , , , , , , , Subject: Re: [PATCH 3/8] memcg, slab: never try to merge memcg caches References: <27c4e7d7fb6b788b66995d2523225ef2dcbc6431.1391356789.git.vdavydov@parallels.com> <20140204145210.GH4890@dhcp22.suse.cz> In-Reply-To: <20140204145210.GH4890@dhcp22.suse.cz> 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 02/04/2014 06:52 PM, Michal Hocko wrote: > On Sun 02-02-14 20:33:48, Vladimir Davydov wrote: >> Suppose we are creating memcg cache A that could be merged with cache B >> of the same memcg. Since any memcg cache has the same parameters as its >> parent cache, parent caches PA and PB of memcg caches A and B must be >> mergeable too. That means PA was merged with PB on creation or vice >> versa, i.e. PA = PB. From that it follows that A = B, and we couldn't >> even try to create cache B, because it already exists - a contradiction. > I cannot tell I understand the above but I am totally not sure about the > statement bellow. > >> So let's remove unused code responsible for merging memcg caches. > How come the code was unused? find_mergeable called cache_match_memcg... Oh, sorry for misleading comment. I mean the code handling merging of per-memcg caches is useless, AFAIU: if we find an alias for a per-memcg cache on kmem_cache_create_memcg(), the parent of the found alias must be the same as the parent_cache passed to kmem_cache_create_memcg(), but if it were so, we would never proceed to the memcg cache creation, because the cache we want to create already exists. Thanks. > >> Signed-off-by: Vladimir Davydov >> --- >> mm/slab.h | 21 ++++----------------- >> mm/slab_common.c | 8 +++++--- >> mm/slub.c | 19 +++++++++---------- >> 3 files changed, 18 insertions(+), 30 deletions(-) >> >> diff --git a/mm/slab.h b/mm/slab.h >> index 8184a7cde272..3045316b7c9d 100644 >> --- a/mm/slab.h >> +++ b/mm/slab.h >> @@ -55,12 +55,12 @@ extern void create_boot_cache(struct kmem_cache *, const char *name, >> struct mem_cgroup; >> #ifdef CONFIG_SLUB >> struct kmem_cache * >> -__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size, >> - size_t align, unsigned long flags, void (*ctor)(void *)); >> +__kmem_cache_alias(const char *name, size_t size, size_t align, >> + unsigned long flags, void (*ctor)(void *)); >> #else >> static inline struct kmem_cache * >> -__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size, >> - size_t align, unsigned long flags, void (*ctor)(void *)) >> +__kmem_cache_alias(const char *name, size_t size, size_t align, >> + unsigned long flags, void (*ctor)(void *)) >> { return NULL; } >> #endif >> >> @@ -119,13 +119,6 @@ static inline bool is_root_cache(struct kmem_cache *s) >> return !s->memcg_params || s->memcg_params->is_root_cache; >> } >> >> -static inline bool cache_match_memcg(struct kmem_cache *cachep, >> - struct mem_cgroup *memcg) >> -{ >> - return (is_root_cache(cachep) && !memcg) || >> - (cachep->memcg_params->memcg == memcg); >> -} >> - >> static inline void memcg_bind_pages(struct kmem_cache *s, int order) >> { >> if (!is_root_cache(s)) >> @@ -204,12 +197,6 @@ static inline bool is_root_cache(struct kmem_cache *s) >> return true; >> } >> >> -static inline bool cache_match_memcg(struct kmem_cache *cachep, >> - struct mem_cgroup *memcg) >> -{ >> - return true; >> -} >> - >> static inline void memcg_bind_pages(struct kmem_cache *s, int order) >> { >> } >> diff --git a/mm/slab_common.c b/mm/slab_common.c >> index 152d9b118b7a..a75834bb966d 100644 >> --- a/mm/slab_common.c >> +++ b/mm/slab_common.c >> @@ -200,9 +200,11 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size, >> */ >> flags &= CACHE_CREATE_MASK; >> >> - s = __kmem_cache_alias(memcg, name, size, align, flags, ctor); >> - if (s) >> - goto out_unlock; >> + if (!memcg) { >> + s = __kmem_cache_alias(name, size, align, flags, ctor); >> + if (s) >> + goto out_unlock; >> + } >> >> err = -ENOMEM; >> s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL); >> diff --git a/mm/slub.c b/mm/slub.c >> index 2b1a6970e46f..962abfdfde06 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -3686,9 +3686,8 @@ static int slab_unmergeable(struct kmem_cache *s) >> return 0; >> } >> >> -static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size, >> - size_t align, unsigned long flags, const char *name, >> - void (*ctor)(void *)) >> +static struct kmem_cache *find_mergeable(size_t size, size_t align, >> + unsigned long flags, const char *name, void (*ctor)(void *)) >> { >> struct kmem_cache *s; >> >> @@ -3707,11 +3706,14 @@ static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size, >> if (slab_unmergeable(s)) >> continue; >> >> + if (!is_root_cache(s)) >> + continue; >> + >> if (size > s->size) >> continue; >> >> if ((flags & SLUB_MERGE_SAME) != (s->flags & SLUB_MERGE_SAME)) >> - continue; >> + continue; >> /* >> * Check if alignment is compatible. >> * Courtesy of Adrian Drzewiecki >> @@ -3722,21 +3724,18 @@ static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size, >> if (s->size - size >= sizeof(void *)) >> continue; >> >> - if (!cache_match_memcg(s, memcg)) >> - continue; >> - >> return s; >> } >> return NULL; >> } >> >> struct kmem_cache * >> -__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size, >> - size_t align, unsigned long flags, void (*ctor)(void *)) >> +__kmem_cache_alias(const char *name, size_t size, size_t align, >> + unsigned long flags, void (*ctor)(void *)) >> { >> struct kmem_cache *s; >> >> - s = find_mergeable(memcg, size, align, flags, name, ctor); >> + s = find_mergeable(size, align, flags, name, ctor); >> if (s) { >> s->refcount++; >> /* >> -- >> 1.7.10.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/