From: Kirill Tkhai <ktkhai@virtuozzo.com>
To: Yang Shi <shy828301@gmail.com>,
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
Subject: Re: [v6 PATCH 09/11] mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers
Date: Thu, 4 Feb 2021 12:29:43 +0300 [thread overview]
Message-ID: <656865f5-bb56-4f4c-b88d-ec933a042b4c@virtuozzo.com> (raw)
In-Reply-To: <20210203172042.800474-10-shy828301@gmail.com>
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 <vbabka@suse.cz>
> Signed-off-by: Yang Shi <shy828301@gmail.com>
> ---
> 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().
>
> @@ -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);
>
> kfree(shrinker->nr_deferred);
> shrinker->nr_deferred = NULL;
>
next prev parent reply other threads:[~2021-02-04 9:30 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-03 17:20 [v6 PATCH 0/11] Make shrinker's nr_deferred memcg aware Yang Shi
2021-02-03 17:20 ` [v6 PATCH 01/11] mm: vmscan: use nid from shrink_control for tracepoint Yang Shi
2021-02-04 7:22 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 02/11] mm: vmscan: consolidate shrinker_maps handling code Yang Shi
2021-02-04 7:23 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 03/11] mm: vmscan: use shrinker_rwsem to protect shrinker_maps allocation Yang Shi
2021-02-04 7:24 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 04/11] mm: vmscan: remove memcg_shrinker_map_size Yang Shi
2021-02-04 8:01 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 05/11] mm: memcontrol: rename shrinker_map to shrinker_info Yang Shi
2021-02-04 8:03 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 06/11] mm: vmscan: use a new flag to indicate shrinker is registered Yang Shi
2021-02-04 8:15 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 07/11] mm: vmscan: add per memcg shrinker nr_deferred Yang Shi
2021-02-04 8:30 ` Kirill Tkhai
2021-02-04 17:17 ` Yang Shi
2021-02-05 14:37 ` Kirill Tkhai
2021-02-05 16:49 ` Yang Shi
2021-02-03 17:20 ` [v6 PATCH 08/11] mm: vmscan: use per memcg nr_deferred of shrinker Yang Shi
2021-02-04 8:41 ` Kirill Tkhai
2021-02-04 17:23 ` Yang Shi
2021-02-05 14:41 ` Kirill Tkhai
2021-02-05 16:40 ` Yang Shi
2021-02-05 3:12 ` [mm] [confidence: ] 3510a44e0e: WARNING:suspicious_RCU_usage kernel test robot
2021-02-03 17:20 ` [v6 PATCH 09/11] mm: vmscan: don't need allocate shrinker->nr_deferred for memcg aware shrinkers Yang Shi
2021-02-04 9:29 ` Kirill Tkhai [this message]
2021-02-04 10:14 ` Kirill Tkhai
2021-02-04 17:32 ` Yang Shi
2021-02-05 14:44 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 10/11] mm: memcontrol: reparent nr_deferred when memcg offline Yang Shi
2021-02-04 10:15 ` Kirill Tkhai
2021-02-03 17:20 ` [v6 PATCH 11/11] mm: vmscan: shrink deferred objects proportional to priority Yang Shi
2021-02-04 10:23 ` Kirill Tkhai
2021-02-04 17:29 ` Yang Shi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=656865f5-bb56-4f4c-b88d-ec933a042b4c@virtuozzo.com \
--to=ktkhai@virtuozzo.com \
--cc=akpm@linux-foundation.org \
--cc=david@fromorbit.com \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=shakeelb@google.com \
--cc=shy828301@gmail.com \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).