From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C6F5C00140 for ; Thu, 18 Aug 2022 22:23:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7FD9A8D0005; Thu, 18 Aug 2022 18:23:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D2DF8D0002; Thu, 18 Aug 2022 18:23:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 69BA28D0005; Thu, 18 Aug 2022 18:23:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 5D46F8D0002 for ; Thu, 18 Aug 2022 18:23:28 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 49871419A6 for ; Thu, 18 Aug 2022 22:23:28 +0000 (UTC) X-FDA: 79814140896.10.F8E06E9 Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by imf05.hostedemail.com (Postfix) with ESMTP id 2E422103D6A for ; Thu, 18 Aug 2022 22:11:53 +0000 (UTC) Received: by mail-io1-f53.google.com with SMTP id p184so2111937iod.6 for ; Thu, 18 Aug 2022 15:11:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=Vo2GoUiFJBInyE5nywoyw7Q4fF2jDGsYcZyTxVixXAU=; b=ktky7FG85SxQIN4nGPoPElDkKx88mLw3wSOhoBHn8jWxmqe2yK7ncp+Bvvmn3V1grt xycIcVChwKblGXinRAJLXpbtxlEjIKSqD/gfD7MXe5l/OKOsrowL6Ss2Uil39tMwNijq h8C6FsiCfRg6+Ich5pcuI9uenN/PdX8W/zx6Vu0QeCmi7mLqOD1N1DKpQufzqdkfYgr/ 3RDPWbSm3132FBjqfq4UABa0uRjMtIfp6x59xTSFr4oMmeviIFqtsfyCJWBgTDXycx+9 CczUWh9fST9eB10UZP8a+azRrx7Xe87VKBrQmCRd6EqAJ67mifIYmVGbG5/P+gfvMjci Fm1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=Vo2GoUiFJBInyE5nywoyw7Q4fF2jDGsYcZyTxVixXAU=; b=qZo2jboTmxZWFajs+la+zk2GxH0kvJBInfDbIoC48G/z+E5Y9mAJAR5FravpPnuydn m+OJD8R7EDeFqsl+WzXHG5emZKyllbL7iJpShnOZnz+pO7IR/wXzk+xY1s7gr7xLfIv0 2LSyT1nrJ1yZIAjJBBt9Zh50WfQUtHqN3743Qu7UxMPxpGYZAhyaHCuB8LXZYo9S2Umv frmy9Olm1giPWRulV0yZH6VIlG+dauKUIqAVNpKPYTB0zmtwqm6GEvuwvKQKlSj5UG3c jVuH6Ajc20fdhuh53lVYm/tpBMji3tkglTfhAUHp7+qYEmPBLVsLrHtA49/lUTUZ1qCK bdIg== X-Gm-Message-State: ACgBeo0u2M+e+kZR9WldrjQUj5moxSjf9GZUzvz/vYaU1QPO3MFXlKrW 93aKpPk7C1pXoN1r/54byQZ/0uL2+ilVkPeGbrT6veXG/VjPLw== X-Google-Smtp-Source: AA6agR4dwGmH7QSsDsC7mKd8PkI8InMbAr1OecRWQCqs1PDnSEI/ToevnauSqWVcuaAWqSQa2jJ3U7ZSQt1lvDyFpGc= X-Received: by 2002:a63:5f8e:0:b0:429:c286:4ef7 with SMTP id t136-20020a635f8e000000b00429c2864ef7mr3579505pgb.166.1660855135926; Thu, 18 Aug 2022 13:38:55 -0700 (PDT) MIME-Version: 1.0 References: <20220818143118.17733-1-laoar.shao@gmail.com> <20220818143118.17733-11-laoar.shao@gmail.com> In-Reply-To: <20220818143118.17733-11-laoar.shao@gmail.com> From: Shakeel Butt Date: Thu, 18 Aug 2022 13:38:44 -0700 Message-ID: Subject: Re: [PATCH bpf-next v2 10/12] mm, memcg: Add new helper get_obj_cgroup_from_cgroup To: Yafang Shao Cc: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , kpsingh@kernel.org, Stanislav Fomichev , Hao Luo , jolsa@kernel.org, Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , Tejun Heo , Zefan Li , Cgroups , netdev , bpf , Linux MM Content-Type: text/plain; charset="UTF-8" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660860715; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Vo2GoUiFJBInyE5nywoyw7Q4fF2jDGsYcZyTxVixXAU=; b=JvJQ0FmRyXpmSBufTh1qBKcfUaItOQqzwv2nbC6NWBonX9TASHy47eVwA2sDqX4gb5J/+4 Hk5pcG3zNyph4Loo9m7B+/8q8jJoGmLRKkk8uqJZTY3rXrjtYjthTbWTSZmrb+GVbRlTbu f08zpse8QcnAr/dl7IuxkGRkNxnPP/4= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ktky7FG8; spf=pass (imf05.hostedemail.com: domain of shakeelb@google.com designates 209.85.166.53 as permitted sender) smtp.mailfrom=shakeelb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660860715; a=rsa-sha256; cv=none; b=1jdb5yqLhcW6JJRhMWyqDICfKmUG8rJNis7XCHszXDbwsi6XmreNrD0P3iWvn7nmW/XUhI cW/R47SW0jTLIodflwoZfBjMn15k0CsQ81Vyf8/uSaRt7wRJfPeKs0QZqZr5CXvsYVaUYa 65Z2gNRC9ufPhNjjePT8SZfh75ISTYk= X-Stat-Signature: jf85n1nzcts8k7h4fs33ue6ba986t3kx X-Rspam-User: Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ktky7FG8; spf=pass (imf05.hostedemail.com: domain of shakeelb@google.com designates 209.85.166.53 as permitted sender) smtp.mailfrom=shakeelb@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 2E422103D6A X-HE-Tag: 1660860713-436983 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Aug 18, 2022 at 7:32 AM Yafang Shao wrote: > > We want to open a cgroup directory and pass the fd into kernel, and then > in the kernel we can charge the allocated memory into the open cgroup if it > has valid memory subsystem. In the bpf subsystem, the opened cgroup will > be store as a struct obj_cgroup pointer, so a new helper > get_obj_cgroup_from_cgroup() is introduced. > > It also add a comment on why the helper __get_obj_cgroup_from_memcg() > must be protected by rcu read lock. > > Signed-off-by: Yafang Shao > --- > include/linux/memcontrol.h | 1 + > mm/memcontrol.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 48 insertions(+) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 2f0a611..901a921 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1713,6 +1713,7 @@ static inline void set_shrinker_bit(struct mem_cgroup *memcg, > int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order); > void __memcg_kmem_uncharge_page(struct page *page, int order); > > +struct obj_cgroup *get_obj_cgroup_from_cgroup(struct cgroup *cgrp); > struct obj_cgroup *get_obj_cgroup_from_current(void); > struct obj_cgroup *get_obj_cgroup_from_page(struct page *page); > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 618c366..0409cc4 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2895,6 +2895,14 @@ struct mem_cgroup *mem_cgroup_from_obj(void *p) > return page_memcg_check(folio_page(folio, 0)); > } > > +/* > + * Pls. note that the memg->objcg can be freed after it is deferenced, > + * that can happen when the memcg is changed from online to offline. > + * So this helper must be protected by read rcu lock. > + * > + * After obj_cgroup_tryget(), it is safe to use the objcg outside of the rcu > + * read-side critical section. > + */ The comment is too verbose. My suggestion would be to add WARN_ON_ONCE(!rcu_read_lock_held()) in the function and if you want to add a comment, just say that the caller should have a reference on memcg. > static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg) > { > struct obj_cgroup *objcg = NULL; > @@ -2908,6 +2916,45 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg) > return objcg; > } > > +static struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg) > +{ > + struct obj_cgroup *objcg; > + > + if (memcg_kmem_bypass()) > + return NULL; > + > + rcu_read_lock(); > + objcg = __get_obj_cgroup_from_memcg(memcg); > + rcu_read_unlock(); > + return objcg; > +} > + > +struct obj_cgroup *get_obj_cgroup_from_cgroup(struct cgroup *cgrp) Since this function is exposed to other files, it would be nice to have a comment similar to get_mem_cgroup_from_mm() for this function. I know other get_obj_cgroup variants do not have such a comment yet but let's start with this. > +{ > + struct cgroup_subsys_state *css; > + struct mem_cgroup *memcg; > + struct obj_cgroup *objcg; > + > + rcu_read_lock(); > + css = rcu_dereference(cgrp->subsys[memory_cgrp_id]); > + if (!css || !css_tryget_online(css)) { Any reason to use css_tryget_online() instead of css_tryget()?