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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69F9BC49ED7 for ; Fri, 20 Sep 2019 14:11:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 34D2C20644 for ; Fri, 20 Sep 2019 14:11:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 34D2C20644 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=virtuozzo.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id BD6A46B0003; Fri, 20 Sep 2019 10:11:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B87996B0005; Fri, 20 Sep 2019 10:11:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC53B6B000E; Fri, 20 Sep 2019 10:11:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0248.hostedemail.com [216.40.44.248]) by kanga.kvack.org (Postfix) with ESMTP id 8AD6F6B0003 for ; Fri, 20 Sep 2019 10:11:19 -0400 (EDT) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id 404AF3D15 for ; Fri, 20 Sep 2019 14:11:19 +0000 (UTC) X-FDA: 75955486278.22.beast74_62b99d67abe2b X-HE-Tag: beast74_62b99d67abe2b X-Filterd-Recvd-Size: 3589 Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) by imf22.hostedemail.com (Postfix) with ESMTP for ; Fri, 20 Sep 2019 14:11:18 +0000 (UTC) Received: from [172.16.24.104] by relay.sw.ru with esmtp (Exim 4.92.2) (envelope-from ) id 1iBJct-0006XP-IK; Fri, 20 Sep 2019 17:11:11 +0300 Subject: Re: [PATCH] mm, memcg: assign shrinker_map before kvfree To: Cyrill Gorcunov , LKML Cc: Linux MM , Johannes Weiner , Michal Hocko , Vladimir Davydov References: <20190920122907.GG2507@uranus.lan> From: Kirill Tkhai Message-ID: <8a4b5293-6f79-2a5d-4ac8-f8fc17f13b6e@virtuozzo.com> Date: Fri, 20 Sep 2019 17:11:00 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20190920122907.GG2507@uranus.lan> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 20.09.2019 15:29, Cyrill Gorcunov wrote: > Currently there is a small gap between fetching pointer, calling > kvfree and assign its value to nil. In current callgraph it is > not a problem (since memcg_free_shrinker_maps is running from > memcg_alloc_shrinker_maps and mem_cgroup_css_free only) still > this looks suspicious and we can easily eliminate the gap at all. > > Cc: Johannes Weiner > Cc: Michal Hocko > Cc: Vladimir Davydov > Cc: Kirill Tkhai > Signed-off-by: Cyrill Gorcunov > --- > mm/memcontrol.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-tip.git/mm/memcontrol.c > =================================================================== > --- linux-tip.git.orig/mm/memcontrol.c > +++ linux-tip.git/mm/memcontrol.c > @@ -364,9 +364,9 @@ static void memcg_free_shrinker_maps(str > for_each_node(nid) { > pn = mem_cgroup_nodeinfo(memcg, nid); > map = rcu_dereference_protected(pn->shrinker_map, true); > + rcu_assign_pointer(pn->shrinker_map, NULL); > if (map) > kvfree(map); > - rcu_assign_pointer(pn->shrinker_map, NULL); > } > } The current scheme is following. We allocate shrinker_map in css_online, while normal freeing happens in css_free. The NULLifying of pointer is needed in case of "abnormal freeing", when memcg_free_shrinker_maps() is called from memcg_alloc_shrinker_maps(). The NULLifying guarantees, we won't free pn->shrinker_map twice. There are no races or problems with kvfree() and rcu_assign_pointer() order, because of nobody can reference shrinker_map before memcg is online. In case of this rcu_assign_pointer() confuses, we may just remove is from the function, and call it only on css_free. Something like the below: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 1c4c08b45e44..454303c3aa0f 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -372,7 +372,6 @@ static void memcg_free_shrinker_maps(struct mem_cgroup *memcg) map = rcu_dereference_protected(pn->shrinker_map, true); if (map) kvfree(map); - rcu_assign_pointer(pn->shrinker_map, NULL); } } @@ -389,7 +388,6 @@ static int memcg_alloc_shrinker_maps(struct mem_cgroup *memcg) for_each_node(nid) { map = kvzalloc(sizeof(*map) + size, GFP_KERNEL); if (!map) { - memcg_free_shrinker_maps(memcg); ret = -ENOMEM; break; }