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 6B5BBC25B06 for ; Fri, 12 Aug 2022 00:27:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 996E58E0001; Thu, 11 Aug 2022 20:27:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9458C6B0075; Thu, 11 Aug 2022 20:27:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 80D7A8E0001; Thu, 11 Aug 2022 20:27:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 718206B0073 for ; Thu, 11 Aug 2022 20:27:50 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4262BAC477 for ; Fri, 12 Aug 2022 00:27:50 +0000 (UTC) X-FDA: 79789052700.04.2720260 Received: from mail-vk1-f171.google.com (mail-vk1-f171.google.com [209.85.221.171]) by imf15.hostedemail.com (Postfix) with ESMTP id D068FA0060 for ; Fri, 12 Aug 2022 00:27:48 +0000 (UTC) Received: by mail-vk1-f171.google.com with SMTP id c22so9747512vko.7 for ; Thu, 11 Aug 2022 17:27:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=1EqMW9+sIv7QJbTeZKUHZUM/p01kIgjNDKoKtzHUX6I=; b=M1GNjTX6MjOuuRZWm3CHgm0eqBJRWOE2DYIwkpijn0zr44uLH0SX2pVKsqnu6SKNns crOKzPpxy1RA43nKlwbqOUFzA7HRedMYZ8tQVF/fVGMAyKmhhwsTdSvsU68Ud2Rd1Vvf VGg7iWJFnTpKjk557gHnFc+YZibSg5rVyMlQJaEi3/Ldv14TBB8M/M2PtEJHVInQAn18 t0OPuaERFvAALtrbkGhfJXRFwmuZN2S5HT8whf8Ecld+YIN1EMC8dvvmJ1vMiAv3owSc silpXSxehuEjE0NLkaQfkPb9niGhisKpqBCPa6hiJgda+d+CFnoRwtbCafYrK4a9pelF WX2Q== 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=1EqMW9+sIv7QJbTeZKUHZUM/p01kIgjNDKoKtzHUX6I=; b=ModFz7ATQxbunKfxAPRixJPci75+jSTeSyuo5AiT2qgDRUVr22XlKxOaXdFyI8qlHr p/+SvcEywbjabT0Q1wGreOUyqpQEPrxTGqq/drmj9g0mFTSvfAx8EmNuLBLwtkdj3g/F l65UoKAHp2+to/2Tk9eFvuKrWKVQcnonXUQxaUgDw6FVzrVSxMGwGYu2nzWaCS7VRK01 CbC0lZCGB5rXP5ucAmna5ZbRUV8TfWGCFKh566vf6fbxMCUKY1k0AOsSVjiBMMVtYJaj FlRSe/HNWDHzIXHXHeYqjSiuluvDlDMUc9ZgpSD6In/vJ6GNi0NykE27llS42/+0cUJF NR9Q== X-Gm-Message-State: ACgBeo2ZQAfjt2u3CJIsiU76s7rfdJMpZtZl+fL5ImwicX3ANSWCzywe prtrZgKC5120UdiCbUe5fcYk7+2KCvKE3sFJuII= X-Google-Smtp-Source: AA6agR6Xxa627apoLE5N7GpRkjtgyd4LlWQoOuRcxBEEqPifbL4QHOVP/BOrxS/OJ4GGBJ0emBtj3jLw3QnSmFAygEI= X-Received: by 2002:a1f:1644:0:b0:378:c157:d0f5 with SMTP id 65-20020a1f1644000000b00378c157d0f5mr902289vkw.5.1660264068116; Thu, 11 Aug 2022 17:27:48 -0700 (PDT) MIME-Version: 1.0 References: <20220810151840.16394-1-laoar.shao@gmail.com> <20220810151840.16394-6-laoar.shao@gmail.com> <20220810170706.ikyrsuzupjwt65h7@google.com> <20220811154731.3smhom6v4qy2u6rd@google.com> In-Reply-To: <20220811154731.3smhom6v4qy2u6rd@google.com> From: Yafang Shao Date: Fri, 12 Aug 2022 08:27:09 +0800 Message-ID: Subject: Re: [PATCH bpf-next 05/15] bpf: Fix incorrect mem_cgroup_put To: Shakeel Butt Cc: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin Lau , Song Liu , Yonghong Song , john fastabend , KP Singh , Stanislav Fomichev , Hao Luo , jolsa@kernel.org, Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Andrew Morton , netdev , bpf , Linux MM Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660264068; a=rsa-sha256; cv=none; b=bQ5vuRzgddMB+qocSOb2ZbWjpdg/OsN9/JHziL50xWq4O+Yr4SmKvICOeDf9PsHxGXDtcB WT0Dc0ilCQMIrTLp8FoJaYmIGu7mHBbfMHSbZ+MoRrKdYl+VzCC4uP17kfDQQ+kgAOGptK 4MZB7pN4EJUFkzRFfogWTjumZi9Fu64= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=M1GNjTX6; spf=pass (imf15.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660264068; 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=1EqMW9+sIv7QJbTeZKUHZUM/p01kIgjNDKoKtzHUX6I=; b=ACuoDTdULCYcEKyxVi3USiXJtyndjR2gOzywPsjUkSLo45TqRRAhuD1yYu7Nkqbp1+QstQ usIYeQZ8kFyLXDdNpHAK00THLaIobk//hh1KXawuJjQXhrepgd6laWVrBJW7Hct/JBzJwF bLXdo4DhU7/PnyRIyha4X672O4+Lwws= X-Stat-Signature: bbc3ic5fyz99xnx5agg1nh54aixk437g X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: D068FA0060 Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=M1GNjTX6; spf=pass (imf15.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.221.171 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-HE-Tag: 1660264068-442161 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 11, 2022 at 11:47 PM Shakeel Butt wrote: > > On Thu, Aug 11, 2022 at 10:49:13AM +0800, Yafang Shao wrote: > > On Thu, Aug 11, 2022 at 1:07 AM Shakeel Butt wrote: > > > > > > On Wed, Aug 10, 2022 at 03:18:30PM +0000, Yafang Shao wrote: > > > > The memcg may be the root_mem_cgroup, in which case we shouldn't put it. > > > > > > No, it is ok to put root_mem_cgroup. css_put already handles the root > > > cgroups. > > > > > > > Ah, this commit log doesn't describe the issue clearly. I should improve it. > > The issue is that in bpf_map_get_memcg() it doesn't get the objcg if > > map->objcg is NULL (that can happen if the map belongs to the root > > memcg), so we shouldn't put the objcg if map->objcg is NULL neither in > > bpf_map_put_memcg(). > > Sorry I am still not understanding. We are not 'getting' objcg in > bpf_map_get_memcg(). We are 'getting' memcg from map->objcg and if that > is NULL the function is returning root memcg and putting root memcg is > totally fine. When the map belongs to root_mem_cgroup, the map->objcg is NULL, right ? See also bpf_map_save_memcg() and it describes clearly in the comment - static void bpf_map_save_memcg(struct bpf_map *map) { /* Currently if a map is created by a process belonging to the root * memory cgroup, get_obj_cgroup_from_current() will return NULL. * So we have to check map->objcg for being NULL each time it's * being used. */ map->objcg = get_obj_cgroup_from_current(); } So for the root_mem_cgroup case, bpf_map_get_memcg() will return root_mem_cgroup directly without getting its css, right ? See below, static struct mem_cgroup *bpf_map_get_memcg(const struct bpf_map *map) { if (map->objcg) return get_mem_cgroup_from_objcg(map->objcg); return root_mem_cgroup; // without css_get(&memcg->css); } But it will put the css unconditionally. See below, memcg = bpf_map_get_memcg(map); ... mem_cgroup_put(memcg); So we should put it *conditionally* as well. memcg = bpf_map_get_memcg(map); ... + if (map->objcg) mem_cgroup_put(memcg); Is it clear to you ? > Or are you saying that root_mem_cgroup is NULL? > No -- Regards Yafang