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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 A2D7EC433E0 for ; Mon, 11 Jan 2021 17:09:01 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5B89022BE9 for ; Mon, 11 Jan 2021 17:09:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5B89022BE9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AEA806B00B3; Mon, 11 Jan 2021 12:09:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A9B096B00B5; Mon, 11 Jan 2021 12:09:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D8B66B00B6; Mon, 11 Jan 2021 12:09:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0029.hostedemail.com [216.40.44.29]) by kanga.kvack.org (Postfix) with ESMTP id 7DF446B00B3 for ; Mon, 11 Jan 2021 12:09:00 -0500 (EST) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2A667181AEF10 for ; Mon, 11 Jan 2021 17:09:00 +0000 (UTC) X-FDA: 77694129240.04.earth17_17104022750e Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin04.hostedemail.com (Postfix) with ESMTP id 029198005191 for ; Mon, 11 Jan 2021 17:08:59 +0000 (UTC) X-HE-Tag: earth17_17104022750e X-Filterd-Recvd-Size: 6748 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) by imf44.hostedemail.com (Postfix) with ESMTP for ; Mon, 11 Jan 2021 17:08:59 +0000 (UTC) Received: by mail-ej1-f41.google.com with SMTP id ce23so648757ejb.8 for ; Mon, 11 Jan 2021 09:08:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wSHsnpaTqaEkArYwkMNNFsZmn9AnEAryiNdCrNxhEoE=; b=BN087SlKLrNpiyOXyf60UmGoQ7ZpTK0Qq52MW+BQtHNuR/esIafv1mbnDd8/SGw7rJ 2E9dcSXdZ7TxMWi0xNTvb65aQBHrTjGV6w+tUbn2Qex1Rb2ZDSV/zYGhdIaNik16dwFX jUBU3IITql0WM+ELoixyxR1CDvUyI1Asn7r8ZM8LX8gz4L3xd/Yu+dQabsTqPw4iLZyI uM8Mw1gq62Sum7EB6aXAt0SJK9nkPwhlPOwiC4my3hGszS/4X3hJgOgq0RwvQ6ICwhSp 1r1enI6HcKD6jUK+yGPElDXsrsxplkPWgJgGMjrqYwLxAtitq+HeXbdxS4gVYOqtniTC DcNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wSHsnpaTqaEkArYwkMNNFsZmn9AnEAryiNdCrNxhEoE=; b=BPPULMUOMg2ZNMFMZNbJXecwFOn7l+M0mdNbkn6UIWBdqe6vVcBsOYNHYJWwcxVj4V tdpv6bag2X5OI6CefK5a9geutpC7J7SbI8DcPCFZm3OgukhEMJwbRY8T4b2mgxwhuByE MtRH3PDP4lPZ9UXUrcJgl0K0jLs3wLbZlzvr43pEN/ENRLmv87p16N8qpHWnXeVkYacQ BSiFm3VCogYML5rHdAqvsQ3DuzdghFMFZSS2xi7p75swe7dS3Cxx7HKqa2G2QircmvAi 83kadobUnz5YMXY5WZk6Bt6hWAQyk1XyBx2SKFGVGz3NUvzFyLsjCDRF5Pw/1fOruoWN ncZg== X-Gm-Message-State: AOAM532m6jJnCq5QDd0P37Bt6bGIyExm77/65x8/Yd1dyHGszDVWUNQX GV6bTOHf69wXjAdw59EB00yGMnyo1rzbSnLn7K4= X-Google-Smtp-Source: ABdhPJxFj5JH/o64LnuDkCdMO9Ola5OitTMkeU/UbF9uTC/pV9hUZAAI59khrlylGOAxBlo4tfWNKCaVcO5sUoSdudk= X-Received: by 2002:a17:906:720e:: with SMTP id m14mr297288ejk.161.1610384938231; Mon, 11 Jan 2021 09:08:58 -0800 (PST) MIME-Version: 1.0 References: <20210105225817.1036378-1-shy828301@gmail.com> <20210105225817.1036378-4-shy828301@gmail.com> <56d26993-1577-3747-2d89-1275d92f7a15@virtuozzo.com> In-Reply-To: <56d26993-1577-3747-2d89-1275d92f7a15@virtuozzo.com> From: Yang Shi Date: Mon, 11 Jan 2021 09:08:46 -0800 Message-ID: Subject: Re: [v3 PATCH 03/11] mm: vmscan: use shrinker_rwsem to protect shrinker_maps allocation To: Kirill Tkhai Cc: Roman Gushchin , Shakeel Butt , Dave Chinner , Johannes Weiner , Michal Hocko , Andrew Morton , Linux MM , Linux FS-devel Mailing List , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" 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 Wed, Jan 6, 2021 at 1:55 AM Kirill Tkhai wrote: > > On 06.01.2021 01:58, Yang Shi wrote: > > Since memcg_shrinker_map_size just can be changd under holding shrinker_rwsem > > exclusively, the read side can be protected by holding read lock, so it sounds > > superfluous to have a dedicated mutex. This should not exacerbate the contention > > to shrinker_rwsem since just one read side critical section is added. > > > > Signed-off-by: Yang Shi > > --- > > mm/vmscan.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 9db7b4d6d0ae..ddb9f972f856 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -187,7 +187,6 @@ static DECLARE_RWSEM(shrinker_rwsem); > > #ifdef CONFIG_MEMCG > > > > static int memcg_shrinker_map_size; > > -static DEFINE_MUTEX(memcg_shrinker_map_mutex); > > > > static void memcg_free_shrinker_map_rcu(struct rcu_head *head) > > { > > @@ -200,8 +199,6 @@ static int memcg_expand_one_shrinker_map(struct mem_cgroup *memcg, > > struct memcg_shrinker_map *new, *old; > > int nid; > > > > - lockdep_assert_held(&memcg_shrinker_map_mutex); > > - > > for_each_node(nid) { > > old = rcu_dereference_protected( > > mem_cgroup_nodeinfo(memcg, nid)->shrinker_map, true); > > @@ -250,7 +247,7 @@ int memcg_alloc_shrinker_maps(struct mem_cgroup *memcg) > > if (mem_cgroup_is_root(memcg)) > > return 0; > > > > - mutex_lock(&memcg_shrinker_map_mutex); > > + down_read(&shrinker_rwsem); > > size = memcg_shrinker_map_size; > > for_each_node(nid) { > > map = kvzalloc(sizeof(*map) + size, GFP_KERNEL); > > @@ -261,7 +258,7 @@ int memcg_alloc_shrinker_maps(struct mem_cgroup *memcg) > > } > > rcu_assign_pointer(memcg->nodeinfo[nid]->shrinker_map, map); > > Here we do STORE operation, and since we want the assignment is visible > for shrink_slab_memcg() under down_read(), we have to use down_write() > in memcg_alloc_shrinker_maps(). I apologize for the late reply, these emails went to my SPAM again. Before this patch it was not serialized by any lock either, right? Do we have to serialize it? As Johannes mentioned if shrinker_maps has not been initialized yet, it means the memcg is a newborn, there should not be significant amount of reclaimable slab caches, so it is fine to skip it. The point makes some sense to me. So, the read lock seems good enough. > > > } > > - mutex_unlock(&memcg_shrinker_map_mutex); > > + up_read(&shrinker_rwsem); > > > > return ret; > > } > > @@ -276,9 +273,8 @@ static int memcg_expand_shrinker_maps(int new_id) > > if (size <= old_size) > > return 0; > > > > - mutex_lock(&memcg_shrinker_map_mutex); > > if (!root_mem_cgroup) > > - goto unlock; > > + goto out; > > > > memcg = mem_cgroup_iter(NULL, NULL, NULL); > > do { > > @@ -287,13 +283,13 @@ static int memcg_expand_shrinker_maps(int new_id) > > ret = memcg_expand_one_shrinker_map(memcg, size, old_size); > > if (ret) { > > mem_cgroup_iter_break(NULL, memcg); > > - goto unlock; > > + goto out; > > } > > } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL); > > -unlock: > > +out: > > if (!ret) > > memcg_shrinker_map_size = size; > > - mutex_unlock(&memcg_shrinker_map_mutex); > > + > > return ret; > > } > > > > > >