On Wed 21-02-18 14:37:56, Shakeel Butt wrote:
[...]
> +#ifdef CONFIG_MEMCG
> +static inline struct mem_cgroup *memalloc_memcg_save(struct mem_cgroup *memcg)
> +{
> + struct mem_cgroup *old_memcg = current->target_memcg;
> + current->target_memcg = memcg;
> + return old_memcg;
> +}
So you are relying that the caller will handle the reference counting
properly? I do not think this is a good idea. Also do we need some kind
of debugging facility to detect unbalanced save/restore scopes?
[...]
> @@ -2260,7 +2269,10 @@ struct kmem_cache *memcg_kmem_get_cache(struct kmem_cache *cachep)
> if (current->memcg_kmem_skip_account)
> return cachep;
>
> - memcg = get_mem_cgroup_from_mm(current->mm);
> + if (current->target_memcg)
> + memcg = get_mem_cgroup(current->target_memcg);
> + if (!memcg)
> + memcg = get_mem_cgroup_from_mm(current->mm);
> kmemcg_id = READ_ONCE(memcg->kmemcg_id);
> if (kmemcg_id < 0)
> goto out;
You are also adding one branch for _each_ charge path even though the
usecase is rather limited.
I will have to think about this approach more. It is clearly less code
than your previous attempt but I cannot say I would be really impressed.
--
Michal Hocko
SUSE Labs
