• Vladimir Davydov's avatar
    memcg: remove KMEM_ACCOUNTED_ACTIVATED flag · 6de64beb
    Vladimir Davydov authored
    Currently we have two state bits in mem_cgroup::kmem_account_flags
    regarding kmem accounting activation, ACTIVATED and ACTIVE.  We start
    kmem accounting only if both flags are set (memcg_can_account_kmem()),
    plus throughout the code there are several places where we check only
    the ACTIVE flag, but we never check the ACTIVATED flag alone.  These
    flags are both set from memcg_update_kmem_limit() under the
    set_limit_mutex, the ACTIVE flag always being set after ACTIVATED, and
    they never get cleared.  That said checking if both flags are set is
    equivalent to checking only for the ACTIVE flag, and since there is no
    ACTIVATED flag checks, we can safely remove the ACTIVATED flag, and
    nothing will change.
    
    Let's try to understand what was the reason for introducing these flags.
    The purpose of the ACTIVE flag is clear - it states that kmem should be
    accounting to the cgroup.  The only requirement for it is that it should
    be set after we have fully initialized kmem accounting bits for the
    cgroup and patched all static branches relating to kmem accounting.
    Since we always check if static branch is enabled before actually
    considering if we should account (otherwise we wouldn't benefit from
    static branching), this guarantees us that we won't skip a commit or
    uncharge after a charge due to an unpatched static branch.
    
    Now let's move on to the ACTIVATED bit.  As I proved in the beginning of
    this message, it is absolutely useless, and removing it will change
    nothing.  So what was the reason introducing it?
    
    The ACTIVATED flag was introduced by commit a8964b9b ("memcg: use
    static branches when code not in use") in order to guarantee that
    static_key_slow_inc(&memcg_kmem_enabled_key) would be called only once
    for each memory cgroup when its kmem accounting was activated.  The
    point was that at that time the memcg_update_kmem_limit() function's
    work-flow looked like this:
    
            bool must_inc_static_branch = false;
    
            cgroup_lock();
            mutex_lock(&set_limit_mutex);
            if (!memcg->kmem_account_flags && val != RESOURCE_MAX) {
                    /* The kmem limit is set for the first time */
                    ret = res_counter_set_limit(&memcg->kmem, val);
    
                    memcg_kmem_set_activated(memcg);
                    must_inc_static_branch = true;
            } else
                    ret = res_counter_set_limit(&memcg->kmem, val);
            mutex_unlock(&set_limit_mutex);
            cgroup_unlock();
    
            if (must_inc_static_branch) {
                    /* We can't do this under cgroup_lock */
                    static_key_slow_inc(&memcg_kmem_enabled_key);
                    memcg_kmem_set_active(memcg);
            }
    
    So that without the ACTIVATED flag we could race with other threads
    trying to set the limit and increment the static branching ref-counter
    more than once.  Today we call the whole memcg_update_kmem_limit()
    function under the set_limit_mutex and this race is impossible.
    
    As now we understand why the ACTIVATED bit was introduced and why we
    don't need it now, and know that removing it will change nothing anyway,
    let's get rid of it.
    Signed-off-by: default avatarVladimir Davydov <vdavydov@parallels.com>
    Cc: Michal Hocko <mhocko@suse.cz>
    Cc: Glauber Costa <glommer@gmail.com>
    Cc: Johannes Weiner <hannes@cmpxchg.org>
    Cc: Balbir Singh <bsingharora@gmail.com>
    Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    6de64beb
memcontrol.c 193 KB