Re: [PATCH 3/8] memcg, slab: never try to merge memcg caches

[Vladimir Davydov]

On 02/04/2014 07:11 PM, Michal Hocko wrote:
> On Tue 04-02-14 18:59:23, Vladimir Davydov wrote:
>> 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.
> I am still not sure I understand this correctly. So the outcome of this
> patch is that compatible caches of different memcgs can be merged
> together? Sorry if this is a stupid question but I am not that familiar
> with this area much I am just seeing that cache_match_memcg goes away
> and my understanding of the function is that it should prevent from
> different memcg's caches merging.

Let me try to explain how I understand it.

What is cache merging/aliasing? When we create a cache
(kmem_cache_create()), we first try to find a compatible cache that
already exists and can handle requests from the new cache. If it is, we
do not create any new caches, instead we simply increment the old cache
refcount and return it.

What about memcgs? Currently, it operates in the same way, i.e. on memcg
cache creation we also try to find a compatible cache of the same memcg
first. But if there were such a cache, they parents would have been
merged (i.e. it would be the same cache). That means we would not even
get to this memcg cache creation, because it already exists. That's why
the code handling memcg caches merging seems pointless to me.

What does this patch change? Actually, it introduces no functional
changes - it only remove the code trying to find an alias for a memcg
cache, because it will fail anyway. So this is rather a cleanup.

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/