Messages in this thread | | | Date | Mon, 22 Sep 2014 18:14:20 +0400 | From | Vladimir Davydov <> | Subject | Re: [PATCH 1/2] memcg: move memcg_{alloc,free}_cache_params to slab_common.c |
| |
On Mon, Sep 22, 2014 at 03:52:45PM +0200, Michal Hocko wrote: > On Thu 18-09-14 19:50:19, Vladimir Davydov wrote: > > The only reason why they live in memcontrol.c is that we get/put css > > reference to the owner memory cgroup in them. However, we can do that in > > memcg_{un,}register_cache. > > > > So let's move them to slab_common.c and make them static. > > Why is it better?
First, I think that the less public interface functions we have in memcontrol.h the better. Since the functions I move don't depend on memcontrol, I think it's worth making them private to slab, especially taking into account that the arrays are defined on the slab's side too.
Second, the way how per-memcg arrays are updated looks rather awkward: it proceeds from memcontrol.c (__memcg_activate_kmem) to slab_common.c (memcg_update_all_caches) and back to memcontrol.c again (memcg_update_array_size). In the next patch I move the function relocating the arrays (memcg_update_array_size) to slab_common.c and therefore get rid this circular call path. I think we should have the cache allocation stuff in the same place where we have relocation, because it's easier to follow the code then. So I move arrays alloc/free functions to slab_common.c too.
The third point isn't obvious. In the "Per memcg slab shrinkers" patch set, which I sent recently, I have to update per-memcg list_lrus arrays too. And it's much easier and cleaner to do it in list_lru.c rather than in memcontrol.c. The current patchset makes cache arrays allocation path conform that of the upcoming list_lru.
Thanks, Vladimir
| |