Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 1 Jun 2022 19:34:38 +0200
From:   Michal =?iso-8859-1?Q?Koutn=FD?= <mkoutny@suse.com>
To:     Muchun Song <songmuchun@bytedance.com>
Cc:     hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev,
        shakeelb@google.com, akpm@linux-foundation.org,
        cgroups@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, duanxiongchun@bytedance.com,
        longman@redhat.com
Subject: Re: [PATCH v5 09/11] mm: memcontrol: use obj_cgroup APIs to charge
 the LRU pages
Message-ID: <20220601173438.GC2434@blackbody.suse.cz>
References: <20220530074919.46352-1-songmuchun@bytedance.com>
 <20220530074919.46352-10-songmuchun@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220530074919.46352-10-songmuchun@bytedance.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Hello Muchun.

On Mon, May 30, 2022 at 03:49:17PM +0800, Muchun Song <songmuchun@bytedance.com> wrote:
> +static inline bool obj_cgroup_is_root(struct obj_cgroup *objcg)
> +{
> +	return objcg == root_obj_cgroup;
> +}

Admittedly, this predicate alone caught my eye, why it did not also
check root_mem_cgroup->objcg_list.
However, deeper look reveals it's purpose is to avoid missing uncharges
of pages that were charged in non-root memcg and later re-associated
upwards after reparenting.
So it's like obj_cgroup_root_origin() (I'm not suggesting a rename, just
illustrating the understanding).

get_obj_cgroup_from_current() gains some complexity but it still holds
that in root memcg neither kernel objects nor LRU pages are charged.
At the same time, reparented kernel objects or LRU pages are properly
uncharged.

These parts are 
Reviewed-by: Michal Koutn? <mkoutny@suse.com>

(I did not look into the locking guarantees with the new API though.)