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 9CC3CC433DB for ; Fri, 5 Feb 2021 16:41:10 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E4E2564ED9 for ; Fri, 5 Feb 2021 16:41:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E4E2564ED9 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 4A0256B0071; Fri, 5 Feb 2021 11:41:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 450716B0074; Fri, 5 Feb 2021 11:41:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3663B6B0075; Fri, 5 Feb 2021 11:41:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0104.hostedemail.com [216.40.44.104]) by kanga.kvack.org (Postfix) with ESMTP id 21DBD6B0071 for ; Fri, 5 Feb 2021 11:41:09 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id D4F3B180AD81D for ; Fri, 5 Feb 2021 16:41:08 +0000 (UTC) X-FDA: 77784779016.24.bee31_3a12f76275e6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin24.hostedemail.com (Postfix) with ESMTP id B6D6A1A4A0 for ; Fri, 5 Feb 2021 16:41:08 +0000 (UTC) X-HE-Tag: bee31_3a12f76275e6 X-Filterd-Recvd-Size: 11968 Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by imf03.hostedemail.com (Postfix) with ESMTP for ; Fri, 5 Feb 2021 16:41:08 +0000 (UTC) Received: by mail-ed1-f54.google.com with SMTP id c6so9537993ede.0 for ; Fri, 05 Feb 2021 08:41:08 -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=hPAfhUCESjqYxvhklcWSyjS56rINMVKFprAe6haqawU=; b=rXFe1jhCZeNcdBQhuNY4eZo7BsFOPNT9L/SeY1Il1LFvC5cRCZno6NhfHj7FJU2/aS ZpkOyBQA83Gzvr1roqPK0C2VPfPJ4P7qzogYHxGTJIZjX6wo1n6A976sQTd4nnodQXdi Neu1bYFA/oBqHSlCbsgJFj/24TLw/FKc9fl7FNiZHJ6qUfqFW8aXyiDf3M82QMWfDwKx ao9TgB3qk6EnVHolvpvVHjynOdD8FrAII075zLTP6CA6pkQ2uezXZFTPhdo4JRpeXrOo hXCGpadCpsTRD5r5Wv9Ohzv24WPMmNzlgqmV2sshp4+i0NTR09Iz64N9XvJX/p/hQAiV 3CfA== 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=hPAfhUCESjqYxvhklcWSyjS56rINMVKFprAe6haqawU=; b=W9GACb7YgapXvEpV8nwrYjEwr+1x3a2dQrRTnDcQ8HBlHGVDeK50riNTkK6XlmiD6s NZG+SR0c9jWBpaP4VlMtqUi4w0BZShg/kA3Rp0brp/s10Ea9K3yDQUXt0ZVFSogfSUt4 R+XnPSx8uos56bnvSZzbxVbl3rJpPgeI9hItmTA46aZsY0KlzGIGaZqqYcR101ypTw2M HH+P2THK30rgewqk5zUgyypQEhddG6sKK9VD20Nxb98B4Sqt/XIiEdx266BuxBSXmaBK zKzEV0Scfb6oEH4s2BBCppnDqHh0QAsUmSsR7EJKFnuOjO79qQjjTDyFfxt393tsBWPZ dCyg== X-Gm-Message-State: AOAM533Z0IRz48owtK9i0B1l1j/5nIYCH0zGgSsIkevKj6b3e4ZoZvUq T1yT64tJvDVP+s7xCvRHOo/gGE5KcaDSD3IPSn8= X-Google-Smtp-Source: ABdhPJy01/k0tVwbX+Xs5TBFDIZkLFpNMKVuWBQrv7nsSZ6X7Y8KqfscMem94WPBqFn4/q4K4rcTUioRNWEiI2HImYc= X-Received: by 2002:a05:6402:3069:: with SMTP id bs9mr2945760edb.151.1612543266826; Fri, 05 Feb 2021 08:41:06 -0800 (PST) MIME-Version: 1.0 References: <20210203172042.800474-1-shy828301@gmail.com> <20210203172042.800474-9-shy828301@gmail.com> <44cc18d2-5a47-91d0-dad2-599c251a3a8b@virtuozzo.com> In-Reply-To: From: Yang Shi Date: Fri, 5 Feb 2021 08:40:55 -0800 Message-ID: Subject: Re: [v6 PATCH 08/11] mm: vmscan: use per memcg nr_deferred of shrinker To: Kirill Tkhai Cc: Roman Gushchin , Vlastimil Babka , 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 Fri, Feb 5, 2021 at 6:42 AM Kirill Tkhai wrote: > > On 04.02.2021 20:23, Yang Shi wrote: > > On Thu, Feb 4, 2021 at 12:42 AM Kirill Tkhai wrote: > >> > >> On 03.02.2021 20:20, Yang Shi wrote: > >>> Use per memcg's nr_deferred for memcg aware shrinkers. The shrinker's nr_deferred > >>> will be used in the following cases: > >>> 1. Non memcg aware shrinkers > >>> 2. !CONFIG_MEMCG > >>> 3. memcg is disabled by boot parameter > >>> > >>> Signed-off-by: Yang Shi > >>> --- > >>> mm/vmscan.c | 94 +++++++++++++++++++++++++++++++++++++++++++---------- > >>> 1 file changed, 77 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/mm/vmscan.c b/mm/vmscan.c > >>> index d9126f12890f..545422d2aeec 100644 > >>> --- a/mm/vmscan.c > >>> +++ b/mm/vmscan.c > >>> @@ -190,6 +190,13 @@ static int shrinker_nr_max; > >>> #define NR_MAX_TO_SHR_MAP_SIZE(nr_max) \ > >>> (DIV_ROUND_UP(nr_max, BITS_PER_LONG) * sizeof(unsigned long)) > >>> > >>> +static struct shrinker_info *shrinker_info_protected(struct mem_cgroup *memcg, > >>> + int nid) > >>> +{ > >>> + return rcu_dereference_protected(memcg->nodeinfo[nid]->shrinker_info, > >>> + lockdep_is_held(&shrinker_rwsem)); > >>> +} > >> > >> Thanks for the helper. Why not to introduce and become to use it in old places > >> in a separate patch? > > > > What do you mean about "old places"? Where was it introduced in v5 (in > > patch #10)? > > I mean existing places touched by this patch, which became to use the new helper > in this patch: free_shrinker_info(), expand_one_shrinker_info(), shrink_slab_memcg(). Aha, I see. So, you mean add the helper before in a separate patch. Right after patch #5 (which rename shrinker_map to shrinker_info) should be a good place. > > >> > >>> + > >>> static void free_shrinker_info_rcu(struct rcu_head *head) > >>> { > >>> kvfree(container_of(head, struct shrinker_info, rcu)); > >>> @@ -204,8 +211,7 @@ static int expand_one_shrinker_info(struct mem_cgroup *memcg, > >>> int size = m_size + d_size; > >>> > >>> for_each_node(nid) { > >>> - old = rcu_dereference_protected( > >>> - mem_cgroup_nodeinfo(memcg, nid)->shrinker_info, true); > >>> + old = shrinker_info_protected(memcg, nid); > >>> /* Not yet online memcg */ > >>> if (!old) > >>> return 0; > >>> @@ -239,7 +245,7 @@ void free_shrinker_info(struct mem_cgroup *memcg) > >>> > >>> for_each_node(nid) { > >>> pn = mem_cgroup_nodeinfo(memcg, nid); > >>> - info = rcu_dereference_protected(pn->shrinker_info, true); > >>> + info = shrinker_info_protected(memcg, nid); > >>> kvfree(info); > >>> rcu_assign_pointer(pn->shrinker_info, NULL); > >>> } > >>> @@ -358,6 +364,25 @@ static void unregister_memcg_shrinker(struct shrinker *shrinker) > >>> up_write(&shrinker_rwsem); > >>> } > >>> > >>> + > >>> +static long count_nr_deferred_memcg(int nid, struct shrinker *shrinker, > >>> + struct mem_cgroup *memcg) > >>> +{ > >>> + struct shrinker_info *info; > >>> + > >>> + info = shrinker_info_protected(memcg, nid); > >>> + return atomic_long_xchg(&info->nr_deferred[shrinker->id], 0); > >>> +} > >>> + > >>> +static long set_nr_deferred_memcg(long nr, int nid, struct shrinker *shrinker, > >>> + struct mem_cgroup *memcg) > >>> +{ > >>> + struct shrinker_info *info; > >>> + > >>> + info = shrinker_info_protected(memcg, nid); > >>> + return atomic_long_add_return(nr, &info->nr_deferred[shrinker->id]); > >>> +} > >> > >> Names confuse me a little bit. What about xchg_nr_deferred_memcg() and add_nr_deferred_memcg()? > > > > add_nr_deferred_memcg() sounds more self-explained to me. > > > >> > >>> static bool cgroup_reclaim(struct scan_control *sc) > >>> { > >>> return sc->target_mem_cgroup; > >>> @@ -396,6 +421,18 @@ static void unregister_memcg_shrinker(struct shrinker *shrinker) > >>> { > >>> } > >>> > >>> +static long count_nr_deferred_memcg(int nid, struct shrinker *shrinker, > >>> + struct mem_cgroup *memcg) > >>> +{ > >>> + return 0; > >>> +} > >>> + > >>> +static long set_nr_deferred_memcg(long nr, int nid, struct shrinker *shrinker, > >>> + struct mem_cgroup *memcg) > >>> +{ > >>> + return 0; > >>> +} > >>> + > >>> static bool cgroup_reclaim(struct scan_control *sc) > >>> { > >>> return false; > >>> @@ -407,6 +444,39 @@ static bool writeback_throttling_sane(struct scan_control *sc) > >>> } > >>> #endif > >>> > >>> +static long count_nr_deferred(struct shrinker *shrinker, > >>> + struct shrink_control *sc) > >>> +{ > >>> + int nid = sc->nid; > >>> + > >>> + if (!(shrinker->flags & SHRINKER_NUMA_AWARE)) > >>> + nid = 0; > >>> + > >>> + if (sc->memcg && > >>> + (shrinker->flags & SHRINKER_MEMCG_AWARE)) > >>> + return count_nr_deferred_memcg(nid, shrinker, > >>> + sc->memcg); > >>> + > >>> + return atomic_long_xchg(&shrinker->nr_deferred[nid], 0); > >>> +} > >>> + > >>> + > >>> +static long set_nr_deferred(long nr, struct shrinker *shrinker, > >>> + struct shrink_control *sc) > >>> +{ > >>> + int nid = sc->nid; > >>> + > >>> + if (!(shrinker->flags & SHRINKER_NUMA_AWARE)) > >>> + nid = 0; > >>> + > >>> + if (sc->memcg && > >>> + (shrinker->flags & SHRINKER_MEMCG_AWARE)) > >>> + return set_nr_deferred_memcg(nr, nid, shrinker, > >>> + sc->memcg); > >>> + > >>> + return atomic_long_add_return(nr, &shrinker->nr_deferred[nid]); > >>> +} > >>> + > >>> /* > >>> * This misses isolated pages which are not accounted for to save counters. > >>> * As the data only determines if reclaim or compaction continues, it is > >>> @@ -539,14 +609,10 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > >>> long freeable; > >>> long nr; > >>> long new_nr; > >>> - int nid = shrinkctl->nid; > >>> long batch_size = shrinker->batch ? shrinker->batch > >>> : SHRINK_BATCH; > >>> long scanned = 0, next_deferred; > >>> > >>> - if (!(shrinker->flags & SHRINKER_NUMA_AWARE)) > >>> - nid = 0; > >>> - > >>> freeable = shrinker->count_objects(shrinker, shrinkctl); > >>> if (freeable == 0 || freeable == SHRINK_EMPTY) > >>> return freeable; > >>> @@ -556,7 +622,7 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > >>> * and zero it so that other concurrent shrinker invocations > >>> * don't also do this scanning work. > >>> */ > >>> - nr = atomic_long_xchg(&shrinker->nr_deferred[nid], 0); > >>> + nr = count_nr_deferred(shrinker, shrinkctl); > >>> > >>> total_scan = nr; > >>> if (shrinker->seeks) { > >>> @@ -647,14 +713,9 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, > >>> next_deferred = 0; > >>> /* > >>> * move the unused scan count back into the shrinker in a > >>> - * manner that handles concurrent updates. If we exhausted the > >>> - * scan, there is no need to do an update. > >>> + * manner that handles concurrent updates. > >>> */ > >>> - if (next_deferred > 0) > >>> - new_nr = atomic_long_add_return(next_deferred, > >>> - &shrinker->nr_deferred[nid]); > >>> - else > >>> - new_nr = atomic_long_read(&shrinker->nr_deferred[nid]); > >>> + new_nr = set_nr_deferred(next_deferred, shrinker, shrinkctl); > >>> > >>> trace_mm_shrink_slab_end(shrinker, shrinkctl->nid, freed, nr, new_nr, total_scan); > >>> return freed; > >>> @@ -674,8 +735,7 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid, > >>> if (!down_read_trylock(&shrinker_rwsem)) > >>> return 0; > >>> > >>> - info = rcu_dereference_protected(memcg->nodeinfo[nid]->shrinker_info, > >>> - true); > >>> + info = shrinker_info_protected(memcg, nid); > >>> if (unlikely(!info)) > >>> goto unlock; > >>> > >>> > >> > >> > >