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=-15.4 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,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 BAF62C433DB for ; Thu, 4 Feb 2021 10:14:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5250364F59 for ; Thu, 4 Feb 2021 10:14:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5250364F59 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine 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 A62736B0005; Thu, 4 Feb 2021 05:14:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EB656B006C; Thu, 4 Feb 2021 05:14:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B40E6B006E; Thu, 4 Feb 2021 05:14:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0141.hostedemail.com [216.40.44.141]) by kanga.kvack.org (Postfix) with ESMTP id 758B36B0005 for ; Thu, 4 Feb 2021 05:14:54 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 305D7181AEF21 for ; Thu, 4 Feb 2021 10:14:54 +0000 (UTC) X-FDA: 77780176908.05.room00_45118e6275db Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 16DEA18014EA8 for ; Thu, 4 Feb 2021 10:14:54 +0000 (UTC) X-HE-Tag: room00_45118e6275db X-Filterd-Recvd-Size: 5256 Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Thu, 4 Feb 2021 10:14:53 +0000 (UTC) Received: from [192.168.15.247] by relay.sw.ru with esmtp (Exim 4.94) (envelope-from ) id 1l7beN-001fd2-Of; Thu, 04 Feb 2021 13:14:11 +0300 Subject: Re: [v6 PATCH 09/11] mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers From: Kirill Tkhai To: Yang Shi , guro@fb.com, vbabka@suse.cz, shakeelb@google.com, david@fromorbit.com, hannes@cmpxchg.org, mhocko@suse.com, akpm@linux-foundation.org Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210203172042.800474-1-shy828301@gmail.com> <20210203172042.800474-10-shy828301@gmail.com> <656865f5-bb56-4f4c-b88d-ec933a042b4c@virtuozzo.com> Message-ID: <5e335e4a-1556-e694-8f0b-192d924f99e5@virtuozzo.com> Date: Thu, 4 Feb 2021 13:14:12 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <656865f5-bb56-4f4c-b88d-ec933a042b4c@virtuozzo.com> 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 04.02.2021 12:29, Kirill Tkhai wrote: > On 03.02.2021 20:20, Yang Shi wrote: >> Now nr_deferred is available on per memcg level for memcg aware shrinkers, so don't need >> allocate shrinker->nr_deferred for such shrinkers anymore. >> >> The prealloc_memcg_shrinker() would return -ENOSYS if !CONFIG_MEMCG or memcg is disabled >> by kernel command line, then shrinker's SHRINKER_MEMCG_AWARE flag would be cleared. >> This makes the implementation of this patch simpler. >> >> Acked-by: Vlastimil Babka >> Signed-off-by: Yang Shi >> --- >> mm/vmscan.c | 31 ++++++++++++++++--------------- >> 1 file changed, 16 insertions(+), 15 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index 545422d2aeec..20a35d26ae12 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -334,6 +334,9 @@ static int prealloc_memcg_shrinker(struct shrinker *shrinker) >> { >> int id, ret = -ENOMEM; >> >> + if (mem_cgroup_disabled()) >> + return -ENOSYS; >> + >> down_write(&shrinker_rwsem); >> /* This may call shrinker, so it must use down_read_trylock() */ >> id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL); >> @@ -414,7 +417,7 @@ static bool writeback_throttling_sane(struct scan_control *sc) >> #else >> static int prealloc_memcg_shrinker(struct shrinker *shrinker) >> { >> - return 0; >> + return -ENOSYS; >> } >> >> static void unregister_memcg_shrinker(struct shrinker *shrinker) >> @@ -525,8 +528,18 @@ unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone >> */ >> int prealloc_shrinker(struct shrinker *shrinker) >> { >> - unsigned int size = sizeof(*shrinker->nr_deferred); >> + unsigned int size; >> + int err; >> + >> + if (shrinker->flags & SHRINKER_MEMCG_AWARE) { >> + err = prealloc_memcg_shrinker(shrinker); >> + if (err != -ENOSYS) >> + return err; >> >> + shrinker->flags &= ~SHRINKER_MEMCG_AWARE; >> + } >> + >> + size = sizeof(*shrinker->nr_deferred); >> if (shrinker->flags & SHRINKER_NUMA_AWARE) >> size *= nr_node_ids; > > This may sound surprisingly, but IIRC do_shrink_slab() may be called on early boot > *even before* root_mem_cgroup is allocated. AFAIR, I received syzcaller crash report > because of this, when I was implementing shrinker_maps. > > This is a reason why we don't use shrinker_maps even in case of mem cgroup is not > disabled: we iterate every shrinker of shrinker_list. See check in shrink_slab(): > > if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg)) > > Possible, we should do the same for nr_deferred: 1)always allocate shrinker->nr_deferred, > 2)use shrinker->nr_deferred in count_nr_deferred() and set_nr_deferred(). I looked over my mail box, and I can't find that crash report and conditions to reproduce. Hm, let's remain this as is, and we rework this in case of such early shrinker call is still possible, and there will be a report... Reviewed-by: Kirill Tkhai With only nit: >> >> @@ -534,26 +547,14 @@ int prealloc_shrinker(struct shrinker *shrinker) >> if (!shrinker->nr_deferred) >> return -ENOMEM; >> >> - if (shrinker->flags & SHRINKER_MEMCG_AWARE) { >> - if (prealloc_memcg_shrinker(shrinker)) >> - goto free_deferred; >> - } >> >> return 0; >> - >> -free_deferred: >> - kfree(shrinker->nr_deferred); >> - shrinker->nr_deferred = NULL; >> - return -ENOMEM; >> } >> >> void free_prealloced_shrinker(struct shrinker *shrinker) >> { >> - if (!shrinker->nr_deferred) >> - return; >> - >> if (shrinker->flags & SHRINKER_MEMCG_AWARE) >> - unregister_memcg_shrinker(shrinker); >> + return unregister_memcg_shrinker(shrinker); I've never seen return of void function in linux kernel. I'm not sure this won't confuse people. >> >> kfree(shrinker->nr_deferred); >> shrinker->nr_deferred = NULL; >> >