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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB937C00528 for ; Tue, 8 Aug 2023 02:24:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 662CB6B0071; Mon, 7 Aug 2023 22:24:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5EBF18D0002; Mon, 7 Aug 2023 22:24:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4653A8D0001; Mon, 7 Aug 2023 22:24:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 2D6336B0071 for ; Mon, 7 Aug 2023 22:24:17 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 05357802B1 for ; Tue, 8 Aug 2023 02:24:16 +0000 (UTC) X-FDA: 81099342954.30.578F1AE Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf12.hostedemail.com (Postfix) with ESMTP id 0FDD140003 for ; Tue, 8 Aug 2023 02:24:14 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b="E8R/sNcx"; dmarc=pass (policy=quarantine) header.from=fromorbit.com; spf=pass (imf12.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=david@fromorbit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691461455; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HUGk8Gm4qWKcvYyYW1oMdtOdnxgdWZtLrf0WmvG8aOg=; b=qiyiM8wXY3HF4K1oNr+0Fgj4PnT63YOa+68buZBIWSLR1p9n+fQ+atkFmUiahRziqErMrI 8YNw2KHIUPktfongHRICcsa583DXaSABXKGRy7+QyFDsJy9uIUXtkbJihIP1SUHSsyiQ4I oybhwavT6HgODi01Lh7EIwOwRKVCMpM= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b="E8R/sNcx"; dmarc=pass (policy=quarantine) header.from=fromorbit.com; spf=pass (imf12.hostedemail.com: domain of david@fromorbit.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=david@fromorbit.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691461455; a=rsa-sha256; cv=none; b=3oOcoIhECDSxvRmUR/5y3YNdhgkxvM2Vw8xNYXtgOR2w1n6BNGV7nIH7IlCpvZ1OT49wH+ lPPF60UZvYjK9OE8Yzzehv6DsBTYCLE5cvRYG02FKaCzYHXHVNUms5FQjFJJm679GJULX4 j4dt8o0UkEO1BkmRv91E7DXU2qsyo+E= Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-1b8ad8383faso46445115ad.0 for ; Mon, 07 Aug 2023 19:24:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20221208.gappssmtp.com; s=20221208; t=1691461454; x=1692066254; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=HUGk8Gm4qWKcvYyYW1oMdtOdnxgdWZtLrf0WmvG8aOg=; b=E8R/sNcxrOK1IUnWWdPxuHAURfzEKsQXFvPF/8WoYIocyBzrOF6C/15wk0bBTyq8fp w+9skh2POsCjChG96SdxCE1ukUJoo7U6iUCJoOHux3qHabiPQt2q8GYhzVnnqh4Ia8gf otMrpF/u+4+6GZvnjfH9iZQVLX7oYyO80eB3aSg/65gqSdDxQ6TBQ0eTbg/7hck8CRIR N2poqJrxPUv+xHM8+kwldwyyrpY8ClA2gjcNqxK3I6Rjaxj3f+CIGTw2aTNSrwyrBhAC fS1NWvmq7XKVFcVROA15iTu7BqzTlMOOgRsfusR61dD9uCRFfNfvH5SixHmdTB0ZZ2at NUZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691461454; x=1692066254; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=HUGk8Gm4qWKcvYyYW1oMdtOdnxgdWZtLrf0WmvG8aOg=; b=PzKa14axMQ/fNdMeM3g4zpTg5J9UJLGqbP8LnEsN3kzp/FrmKaj6l1xFDHblNbbR2S 5nLcljb6UP+iOoGjmeuNR4CcO0tO8Obtd9weCxYK0VP6BLkvrqmqc6YAJ71USbDKiYY8 uh/efva3wEZfX/olZo3OfZFet00cZy3DLdOY50QX9v9iZ0YD5Gj8E5clxvhf5bZHgelT iM2F7fakJo2ZO+mi8sM+9ESU/PDLDdKrxmw/AUczDNBkEXwvYOm2DnlsBeWWE3B6O7+4 ewbYuwYmXUXEKTzsRGQreLZ/jk1bEE39j6Fxgm4dwta1A+F8MCu5v3K0Nop+LzuoqWHD G2cQ== X-Gm-Message-State: AOJu0Yz0Tno4pSdEGfWe6SDRCauf7RiHYIFKgCWM5GuknGn/8biDm5xT mmK60/mn7ehm5xKq5SYYjKf7gg== X-Google-Smtp-Source: AGHT+IEzrDGpXyLCXRNBmZGxK80nBmbnZ3Zc29NdEkVzSJ3QNXEJvOFh13D52+77LsVxnWz7HKqTHA== X-Received: by 2002:a17:902:8692:b0:1b9:d307:c1df with SMTP id g18-20020a170902869200b001b9d307c1dfmr10722096plo.17.1691461453690; Mon, 07 Aug 2023 19:24:13 -0700 (PDT) Received: from dread.disaster.area (pa49-180-166-213.pa.nsw.optusnet.com.au. [49.180.166.213]) by smtp.gmail.com with ESMTPSA id h17-20020a170902f55100b001b864add154sm7583543plf.154.2023.08.07.19.24.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Aug 2023 19:24:13 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1qTCOH-002Wjl-2u; Tue, 08 Aug 2023 12:24:09 +1000 Date: Tue, 8 Aug 2023 12:24:09 +1000 From: Dave Chinner To: Qi Zheng Cc: akpm@linux-foundation.org, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, steven.price@arm.com, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org, muchun.song@linux.dev, simon.horman@corigine.com, dlemoal@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org, kvm@vger.kernel.org, xen-devel@lists.xenproject.org, linux-erofs@lists.ozlabs.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-nfs@vger.kernel.org, linux-mtd@lists.infradead.org, rcu@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, dm-devel@redhat.com, linux-raid@vger.kernel.org, linux-bcache@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH v4 45/48] mm: shrinker: make global slab shrink lockless Message-ID: References: <20230807110936.21819-1-zhengqi.arch@bytedance.com> <20230807110936.21819-46-zhengqi.arch@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230807110936.21819-46-zhengqi.arch@bytedance.com> X-Rspamd-Queue-Id: 0FDD140003 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: dk349ntw535raerebexnk3gthmkit67p X-HE-Tag: 1691461454-595818 X-HE-Meta: U2FsdGVkX18Wsfvs+lvabggC1A3MFEwncB/kE6zJ1hz+YwmZPdSMJ23Qp2Mf0ZZrTJ/hMc47DS5VeB37vAn+qkvZNEuJsgKh1qAWTf3gd9wlK0ipBaLey00o7QGxqfVBuvORX8Mjw+Hp5RKVbUYhjbWDpiTPl0OfjZW6vh3lq4EHiHG9O8tlJmxkL7KZsqwmsa2IeOvPs7sEeey4c7vTZcMO2VcKkw9RmDJT2LcwN/VAWUgLvb0LNGz48gEpfslARe/9IkbCfnZuzPa+ehyScUVbkn3GdBlyQnDGXLi+aFSm4plsEusQe5BWTLR3d91QNe4wYHXXhAwNKmdEh2gjwY9wrgf2IoFbJVCyUx3KJu7oAFFBufRRFp6pkFjpYo8kemCMO3+sd8/97JTDhtlKtmrYXjOsCxW9OsIsLrsnwLkO17hMYOQ4OD5mbltlAQg3AgQ/57C1kEXJ/rohnrFl+6xzc72xVMwUNyPZPEG5JNNwI///GrOOZkkcpbk3ETuaAijSlSD3+IdwaNitczTlo3tDzsEPkuI35dzPAAuW90159Cis9pJSMTL8Ar8XC1uzqcYFcgUQXKaSV7T6sJQuPl2WvWV8NpfK03A3BbUmRX/TK4i7BQq31//TcWmFF+T7Jv5mOQbpmH/iZLoExfJpV4P9dQXlLzzPTNGVkj0j5BMk44OWzVpICR3sYMpV3kvaKcMKp5UTZy/kBLYCT7ljcNG1WepfKtpRIGdLzLmdOaTl3gevLfZEuMb8KTfZFnjQoT63DSRaOXKK/rUbDz2NYfrmgqPVWNirdQ7tk4KjKfAUz7QsgN0jAYYmhkgq2Xz5JPcNRlO39NHHJfsstRw165yj0txypFI2ZfnjqX0RJ779qnliiJtvCd4XePH7JaLGvbvFDr5TXVE2SuiEqT4IAVIbvcONGNoQ2vSDoF6A80O+5y1YgCBnsEiQMYl5WC1qxA9+dSu/vpyp1VpkBA5 uYJo5rWk RN9trKMoJ7lGI5IcZ7nTnxtrw+EmhuEQrcEBRz/3ACbHJP5JxIb9ZEdtXp8S9Ddkjs2R6ly2IrUOvOFDcYP+NW5B35pnSt7zomAXmc5bzpiDY7fqDYM4NsrlekJy4ztT8T0GGNK9582uS/lVV6UcSOlkfSC2ho/mnVAGSqKCoI1LVuV9DkF2r38p3yVzcMXMxRm06ekTObzY+Tj1iytaW4JXx5StRIUFT614+Z/8/I+jlLjj9ODVBYS8f57Cea3vFylSdeozr89M+1dJY8z/P/EI6SDaJG+lmLzDpgAXmLAr4jahcz/f4LDTomGFI7xVxHx86ndwdZVOSW9yYkNxobhcg1HVr63ufkFqZeOJzYfn6xIxJXV8/8moLJaqsyI++GmilydNSMiyTXsXB3m9PnLBOeR+e+KreM+cyD0iPSNDRfkt7zmZlpP7hWkvKiSof177REyMiQNnN2mhV5UB7Dh4ZvrkfyDlHJamBvk3aQt65cF0= 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 Mon, Aug 07, 2023 at 07:09:33PM +0800, Qi Zheng wrote: > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h > index eb342994675a..f06225f18531 100644 > --- a/include/linux/shrinker.h > +++ b/include/linux/shrinker.h > @@ -4,6 +4,8 @@ > > #include > #include > +#include > +#include > > #define SHRINKER_UNIT_BITS BITS_PER_LONG > > @@ -87,6 +89,10 @@ struct shrinker { > int seeks; /* seeks to recreate an obj */ > unsigned flags; > > + refcount_t refcount; > + struct completion done; > + struct rcu_head rcu; Documentation, please. What does the refcount protect, what does the completion provide, etc. > + > void *private_data; > > /* These are for internal use */ > @@ -120,6 +126,17 @@ struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...); > void shrinker_register(struct shrinker *shrinker); > void shrinker_free(struct shrinker *shrinker); > > +static inline bool shrinker_try_get(struct shrinker *shrinker) > +{ > + return refcount_inc_not_zero(&shrinker->refcount); > +} > + > +static inline void shrinker_put(struct shrinker *shrinker) > +{ > + if (refcount_dec_and_test(&shrinker->refcount)) > + complete(&shrinker->done); > +} > + > #ifdef CONFIG_SHRINKER_DEBUG > extern int __printf(2, 3) shrinker_debugfs_rename(struct shrinker *shrinker, > const char *fmt, ...); > diff --git a/mm/shrinker.c b/mm/shrinker.c > index 1911c06b8af5..d318f5621862 100644 > --- a/mm/shrinker.c > +++ b/mm/shrinker.c > @@ -2,6 +2,7 @@ > #include > #include > #include > +#include > #include > > #include "internal.h" > @@ -577,33 +578,42 @@ unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg, > 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) { > + rcu_read_lock(); > + list_for_each_entry_rcu(shrinker, &shrinker_list, list) { > struct shrink_control sc = { > .gfp_mask = gfp_mask, > .nid = nid, > .memcg = memcg, > }; > > + if (!shrinker_try_get(shrinker)) > + continue; > + > + /* > + * We can safely unlock the RCU lock here since we already > + * hold the refcount of the shrinker. > + */ > + rcu_read_unlock(); > + > ret = do_shrink_slab(&sc, shrinker, priority); > 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. > + * This shrinker may be deleted from shrinker_list and freed > + * after the shrinker_put() below, but this shrinker is still > + * used for the next traversal. So it is necessary to hold the > + * RCU lock first to prevent this shrinker from being freed, > + * which also ensures that the next shrinker that is traversed > + * will not be freed (even if it is deleted from shrinker_list > + * at the same time). > */ This needs to be moved to the head of the function, and document the whole list walk, get, put and completion parts of the algorithm that make it safe. There's more to this than "we hold a reference count", especially the tricky "we might see the shrinker before it is fully initialised" case.... ..... > void shrinker_free(struct shrinker *shrinker) > { > struct dentry *debugfs_entry = NULL; > @@ -686,9 +712,18 @@ void shrinker_free(struct shrinker *shrinker) > if (!shrinker) > return; > > + if (shrinker->flags & SHRINKER_REGISTERED) { > + shrinker_put(shrinker); > + wait_for_completion(&shrinker->done); > + } Needs a comment explaining why we need to wait here... > + > down_write(&shrinker_rwsem); > if (shrinker->flags & SHRINKER_REGISTERED) { > - list_del(&shrinker->list); > + /* > + * Lookups on the shrinker are over and will fail in the future, > + * so we can now remove it from the lists and free it. > + */ .... rather than here after the wait has been done and provided the guarantee that no shrinker is running or will run again... -Dave. -- Dave Chinner david@fromorbit.com