I think that all manipulations on memcg_caches array must happen where
it is defined, i.e. on the slab side. The array allocation and
relocation paths as well as elements access follow this pattern (see
e.g. cache_from_memcg_idx, memcg_update_all_caches), but elements update
doesn't. We still want to setup new array elements in memcontrol.c (see
memcg_{un,}register_cache), though it may change in the future. Anyway,
let's introduce a simple function for updating the array entries,
cache_install_at_memcg_idx, to match cache_from_memcg_idx.Signed-off-by: Vladimir Davydov <[email protected]> --- mm/memcontrol.c | 15 ++++----------- mm/slab.h | 21 ++++++++++++++++++++- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 412fa220b9aa..9ae2627bd3b1 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3018,15 +3018,8 @@ static void memcg_register_cache(struct mem_cgroup *memcg, css_get(&memcg->css); list_add(&cachep->memcg_params->list, &memcg->memcg_slab_caches); - /* - * Since readers won't lock (see cache_from_memcg_idx()), we need a - * barrier here to ensure nobody will see the kmem_cache partially - * initialized. - */ - smp_wmb(); - - BUG_ON(root_cache->memcg_params->memcg_caches[id]); - root_cache->memcg_params->memcg_caches[id] = cachep; + BUG_ON(cache_from_memcg_idx(root_cache, id) != NULL); + cache_install_at_memcg_idx(root_cache, id, cachep); } static void memcg_unregister_cache(struct kmem_cache *cachep) @@ -3043,8 +3036,8 @@ static void memcg_unregister_cache(struct kmem_cache *cachep) memcg = cachep->memcg_params->memcg; id = memcg_cache_id(memcg); - BUG_ON(root_cache->memcg_params->memcg_caches[id] != cachep); - root_cache->memcg_params->memcg_caches[id] = NULL; + BUG_ON(cache_from_memcg_idx(root_cache, id) != cachep); + cache_install_at_memcg_idx(root_cache, id, NULL); list_del(&cachep->memcg_params->list); diff --git a/mm/slab.h b/mm/slab.h index 52b570932ba0..da798cfe5efa 100644 --- a/mm/slab.h +++ b/mm/slab.h @@ -201,12 +201,31 @@ cache_from_memcg_idx(struct kmem_cache *s, int idx) /* * Make sure we will access the up-to-date value. The code updating * memcg_caches issues a write barrier to match this (see - * memcg_register_cache()). + * cache_install_at_memcg_idx()). */ smp_read_barrier_depends(); return cachep; } +/* + * Update the entry at index @memcg_idx in the memcg_caches array of + * @root_cache. The caller must synchronize against concurrent updates to the + * same entry as well as guarantee that the memcg_caches array won't be + * relocated under our noses. + */ +static inline void cache_install_at_memcg_idx(struct kmem_cache *root_cache, + int memcg_idx, struct kmem_cache *memcg_cache) +{ + /* + * Since readers won't lock (see cache_from_memcg_idx()), we need a + * barrier here to ensure nobody will see the kmem_cache partially + * initialized. + */ + smp_wmb(); + + root_cache->memcg_params->memcg_caches[memcg_idx] = memcg_cache; +} + static inline struct kmem_cache *memcg_root_cache(struct kmem_cache *s) { if (is_root_cache(s)) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
