From patchwork Mon Sep 27 07:48:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Sultan Alsawaf (unemployed)" X-Patchwork-Id: 12519289 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46E3AC433EF for ; Mon, 27 Sep 2021 07:48:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D24B660F4F for ; Mon, 27 Sep 2021 07:48:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D24B660F4F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kerneltoast.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 53C656B0071; Mon, 27 Sep 2021 03:48:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4EBB76B0072; Mon, 27 Sep 2021 03:48:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3B42F900002; Mon, 27 Sep 2021 03:48:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0237.hostedemail.com [216.40.44.237]) by kanga.kvack.org (Postfix) with ESMTP id 2C97F6B0071 for ; Mon, 27 Sep 2021 03:48:44 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id DC4BE180269F5 for ; Mon, 27 Sep 2021 07:48:43 +0000 (UTC) X-FDA: 78632576526.39.A8DD88F Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by imf24.hostedemail.com (Postfix) with ESMTP id 96642B0000A0 for ; Mon, 27 Sep 2021 07:48:43 +0000 (UTC) Received: by mail-pj1-f43.google.com with SMTP id nn5-20020a17090b38c500b0019af1c4b31fso12795617pjb.3 for ; Mon, 27 Sep 2021 00:48:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=0FGQRpkHFm5jWPBG/X3cVcH6mTWRwTcwTNAv1UtGu6c=; b=ThniXubkp20lKAjoW/YNPcxf6QKlFDV3ptz0bdzgot6I9v8o07y9tg0F1/u/yhC8dj 4Y7rB1lNu8d6Gx//C0L98jEyO1kEcnetL5N4SsRNVh1BNC55NI0Hy7cKpX/J2eD2faql GTVnL8W7TUtKNgxcaBZ/lzdygrpH6V5Cd9RhNgbdhZKjWEYuPUl9uIcBgMsvFxIuf2aY Em1R6wozVnxANCiQMwx4XAP6aucR1T30o/bSZcyxt5XMpiVs5iJMmcSQBxmSZBgJhgGz p0qInyK5lNfYfc40a4bc1XNtRDZnmOc4VeX78sNGuEulFFhCmCmxKXW1ZuuE//9eOim8 gZfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :mime-version:content-transfer-encoding; bh=0FGQRpkHFm5jWPBG/X3cVcH6mTWRwTcwTNAv1UtGu6c=; b=UpmigDEAcBceoKzAyWugRkGz82bVqbYxtwJ7nvjsNvCF8+F9YA4/guBWXRf4gsWTni SbKlet1jLCpmT7LLmrvWStNvs/4DfIiHGP51Wg4sItbpgFecrSS7HnFt0kxQbn8rxm69 V5ZAg6QpHe7phUk2VJpdW6/RJTIdQciPhaC24wymaITcMQJ2RlWs7pvCzcwYxa78sAwB dysVWQ0BFYCQxbuH9vN6CCfv7HsRklGh11eqBadmqcZTisvy5UlflPbJ8dq33rn3ua6Q 9Yeh8VIe59wSaR9jk5ThlWMdANIHtBR0BMY5mynvoY1nalL9avIHUPj+47Q2cQvRn0xv RS1Q== X-Gm-Message-State: AOAM531HlgLPzoAO2KaCY1D/O2nXKVljiS8Mr2RxJcPxUexAbqLyI8nM F/V4ThL2iyal38RLMaVINGo= X-Google-Smtp-Source: ABdhPJwWE0NmrGq1ePwqZUX3w0uM+j+LT17QtpLBWOJbMDxLbBCmeszYxskJ+Fv3A3tM6f0Lu6d+qg== X-Received: by 2002:a17:90a:1991:: with SMTP id 17mr18350509pji.149.1632728922424; Mon, 27 Sep 2021 00:48:42 -0700 (PDT) Received: from sultan-box.localdomain ([204.152.215.247]) by smtp.gmail.com with ESMTPSA id v4sm8447256pjr.32.2021.09.27.00.48.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Sep 2021 00:48:42 -0700 (PDT) From: Sultan Alsawaf X-Google-Original-From: Sultan Alsawaf To: Cc: mhocko@kernel.org, Sultan Alsawaf , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH] mm: vmscan: Replace shrinker_rwsem trylocks with SRCU protection Date: Mon, 27 Sep 2021 00:48:23 -0700 Message-Id: <20210927074823.5825-1-sultan@kerneltoast.com> X-Mailer: git-send-email 2.33.0 MIME-Version: 1.0 X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 96642B0000A0 X-Stat-Signature: 9nwhxopp7ywt3eho7hkierztzb4zy37y Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=ThniXubk; dmarc=none; spf=none (imf24.hostedemail.com: domain of mail-pj1-f43.google.com has no SPF policy when checking 209.85.216.43) smtp.helo=mail-pj1-f43.google.com X-HE-Tag: 1632728923-438981 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: From: Sultan Alsawaf The shrinker rwsem is problematic because the actual shrinking path must back off when contention appears, causing some or all shrinkers to be skipped. This can be especially bad when shrinkers are frequently registered and unregistered. A high frequency of shrinker registrations/ unregistrations can effectively DoS the shrinker mechanism, rendering it useless. Using SRCU to protect iteration through the shrinker list and idr eliminates this issue entirely. Now, shrinking can happen concurrently with shrinker registrations/unregistrations. Signed-off-by: Sultan Alsawaf --- mm/vmscan.c | 68 ++++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/mm/vmscan.c b/mm/vmscan.c index 74296c2d1fed..3dea927be5ad 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -190,6 +190,7 @@ static void set_task_reclaim_state(struct task_struct *task, static LIST_HEAD(shrinker_list); static DECLARE_RWSEM(shrinker_rwsem); +DEFINE_STATIC_SRCU(shrinker_srcu); #ifdef CONFIG_MEMCG static int shrinker_nr_max; @@ -212,6 +213,20 @@ static struct shrinker_info *shrinker_info_protected(struct mem_cgroup *memcg, lockdep_is_held(&shrinker_rwsem)); } +static struct shrinker_info *shrinker_info_srcu(struct mem_cgroup *memcg, + int nid) +{ + return srcu_dereference(memcg->nodeinfo[nid]->shrinker_info, + &shrinker_srcu); +} + +static void free_shrinker_info_srcu(struct rcu_head *head) +{ + struct shrinker_info *info = container_of(head, typeof(*info), rcu); + + kvfree_rcu(info, rcu); +} + static int expand_one_shrinker_info(struct mem_cgroup *memcg, int map_size, int defer_size, int old_map_size, int old_defer_size) @@ -244,7 +259,12 @@ static int expand_one_shrinker_info(struct mem_cgroup *memcg, defer_size - old_defer_size); rcu_assign_pointer(pn->shrinker_info, new); - kvfree_rcu(old, rcu); + + /* + * Shrinker info is used under both SRCU and regular RCU, so + * synchronize the free against both of them. + */ + call_srcu(&shrinker_srcu, &old->rcu, free_shrinker_info_srcu); } return 0; @@ -357,7 +377,6 @@ static int prealloc_memcg_shrinker(struct shrinker *shrinker) 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); if (id < 0) goto unlock; @@ -391,7 +410,7 @@ static long xchg_nr_deferred_memcg(int nid, struct shrinker *shrinker, { struct shrinker_info *info; - info = shrinker_info_protected(memcg, nid); + info = shrinker_info_srcu(memcg, nid); return atomic_long_xchg(&info->nr_deferred[shrinker->id], 0); } @@ -400,7 +419,7 @@ static long add_nr_deferred_memcg(long nr, int nid, struct shrinker *shrinker, { struct shrinker_info *info; - info = shrinker_info_protected(memcg, nid); + info = shrinker_info_srcu(memcg, nid); return atomic_long_add_return(nr, &info->nr_deferred[shrinker->id]); } @@ -641,6 +660,7 @@ void free_prealloced_shrinker(struct shrinker *shrinker) down_write(&shrinker_rwsem); unregister_memcg_shrinker(shrinker); up_write(&shrinker_rwsem); + synchronize_srcu(&shrinker_srcu); return; } @@ -651,7 +671,7 @@ void free_prealloced_shrinker(struct shrinker *shrinker) void register_shrinker_prepared(struct shrinker *shrinker) { down_write(&shrinker_rwsem); - list_add_tail(&shrinker->list, &shrinker_list); + list_add_tail_rcu(&shrinker->list, &shrinker_list); shrinker->flags |= SHRINKER_REGISTERED; up_write(&shrinker_rwsem); } @@ -676,11 +696,12 @@ void unregister_shrinker(struct shrinker *shrinker) return; down_write(&shrinker_rwsem); - list_del(&shrinker->list); + list_del_rcu(&shrinker->list); shrinker->flags &= ~SHRINKER_REGISTERED; if (shrinker->flags & SHRINKER_MEMCG_AWARE) unregister_memcg_shrinker(shrinker); up_write(&shrinker_rwsem); + synchronize_srcu(&shrinker_srcu); kfree(shrinker->nr_deferred); shrinker->nr_deferred = NULL; @@ -792,15 +813,13 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid, { struct shrinker_info *info; unsigned long ret, freed = 0; - int i; + int i, srcu_idx; if (!mem_cgroup_online(memcg)) return 0; - if (!down_read_trylock(&shrinker_rwsem)) - return 0; - - info = shrinker_info_protected(memcg, nid); + srcu_idx = srcu_read_lock(&shrinker_srcu); + info = shrinker_info_srcu(memcg, nid); if (unlikely(!info)) goto unlock; @@ -850,14 +869,9 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid, set_shrinker_bit(memcg, nid, i); } freed += ret; - - if (rwsem_is_contended(&shrinker_rwsem)) { - freed = freed ? : 1; - break; - } } unlock: - up_read(&shrinker_rwsem); + srcu_read_unlock(&shrinker_srcu, srcu_idx); return freed; } #else /* CONFIG_MEMCG */ @@ -894,6 +908,7 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, { unsigned long ret, freed = 0; struct shrinker *shrinker; + int srcu_idx; /* * The root memcg might be allocated even though memcg is disabled @@ -905,10 +920,9 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg)) return shrink_slab_memcg(gfp_mask, nid, memcg, priority); - if (!down_read_trylock(&shrinker_rwsem)) - goto out; - - list_for_each_entry(shrinker, &shrinker_list, list) { + srcu_idx = srcu_read_lock(&shrinker_srcu); + list_for_each_entry_srcu(shrinker, &shrinker_list, list, + srcu_read_lock_held(&shrinker_srcu)) { struct shrink_control sc = { .gfp_mask = gfp_mask, .nid = nid, @@ -919,19 +933,9 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, if (ret == SHRINK_EMPTY) ret = 0; freed += ret; - /* - * Bail out if someone want to register a new shrinker to - * prevent the registration from being stalled for long periods - * by parallel ongoing shrinking. - */ - if (rwsem_is_contended(&shrinker_rwsem)) { - freed = freed ? : 1; - break; - } } + srcu_read_unlock(&shrinker_srcu, srcu_idx); - up_read(&shrinker_rwsem); -out: cond_resched(); return freed; }