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 EB256EB64D7 for ; Fri, 23 Jun 2023 06:12:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4A8988D0002; Fri, 23 Jun 2023 02:12:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 458D98D0001; Fri, 23 Jun 2023 02:12:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3206A8D0002; Fri, 23 Jun 2023 02:12:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 243D98D0001 for ; Fri, 23 Jun 2023 02:12:13 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id E2F351A028A for ; Fri, 23 Jun 2023 06:12:12 +0000 (UTC) X-FDA: 80932992504.26.44FC242 Received: from mail-qk1-f181.google.com (mail-qk1-f181.google.com [209.85.222.181]) by imf22.hostedemail.com (Postfix) with ESMTP id E4B48C000D for ; Fri, 23 Jun 2023 06:12:08 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b=SecpzgOS; dmarc=pass (policy=quarantine) header.from=fromorbit.com; spf=pass (imf22.hostedemail.com: domain of david@fromorbit.com designates 209.85.222.181 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=1687500729; 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=g6Rx1Xf0LH0dvCkca5awQZbD4QgAJXvuKCZBBi/mUhU=; b=Q9X8vzakqCxLeiUhO3hPho5mCD9SiiO6dLxiQzSsVanPywcp7x4dBQeAGP6aW1J2umm6DG h5xL430bsnpO4v5lr13Hn9ou2CZJZwB7UieSjv7NZi7drNoKvzqyGjWRxlw20YtFdZm1j5 w8gu4ju7tysHdSLVGFsxUVQLOLqRMqA= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=fromorbit-com.20221208.gappssmtp.com header.s=20221208 header.b=SecpzgOS; dmarc=pass (policy=quarantine) header.from=fromorbit.com; spf=pass (imf22.hostedemail.com: domain of david@fromorbit.com designates 209.85.222.181 as permitted sender) smtp.mailfrom=david@fromorbit.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687500729; a=rsa-sha256; cv=none; b=pKuV8CJKO7fA60eikRaE297FfWXi6gt7qCz4j59uDkmhKkwqhW3XBqyfG2Q0hoUSvGgaJP 61weFb/eSk7tHfKwUKHUDV6wc1j8YN+a0sFkJaukoIe/Sq7BdgHBv2rv9MYg6LwC5fvIlt 0K9wDTc0/k7dB+TV2B+mu8qTcsHMnLU= Received: by mail-qk1-f181.google.com with SMTP id af79cd13be357-76243a787a7so18731685a.2 for ; Thu, 22 Jun 2023 23:12:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20221208.gappssmtp.com; s=20221208; t=1687500728; x=1690092728; 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=g6Rx1Xf0LH0dvCkca5awQZbD4QgAJXvuKCZBBi/mUhU=; b=SecpzgOSYBos+0sqL4EbPW75FGWVTx9L5r0t3G2iuevcqFrKG4F/tLNs6eWS22kOmh C0PZvcjnf7UYxgnqR+bpG0mTlEGom1YtlVhT3auz/n35x7KxiTrn9uMCHEu7lYc1dP6p eY+aw4EnvIi6B8fviKsKoQH1Jzl15W85hgTxNUcsJ3dNHiRrVQFHxysMeC3SWqeWxnsy yeeVzTshwEBU92s9eN4WsR3lm2qnu+8pne5nYPYtXC9PnDXRC5K4UnNnq5jjmfz2fAMm luXgCfjy27KBowEi38EwnMkC5XGcuX6tyW21/tEwr1FrSp+rGCyLdSsWF3R6DJsSbivl 1Q3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687500728; x=1690092728; 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=g6Rx1Xf0LH0dvCkca5awQZbD4QgAJXvuKCZBBi/mUhU=; b=Tg9alUilPUTsvGU1rayR8Wr75dYqUWPGAsJOP9DkbXnaTFv8AodEIKGb3HyULiazP4 3VVviSBeEkbkxc4i9NU2kutSWS7/NvPthph22WsVI1a702jxNSc1PjvWVnr7sRlzm2nh 90xJuH2EN6+v5Pq1xbCznrq75J9wTmg6dKlUzR8LdQKZZ3NtfJsTpRK6GQ/vRGYUmd3S m2Giw/74zRKRmPfHAB5AWffCZKLxEbkVjqLhbPWtgj5U7Z2gQN4HMj6eS92GU5PaDUUr 7ilpa6p9kubTZuXD9PYy3pzaKc2S6LxnSpnabtxlJhy9OTc4EPlsCL24awBhtmG476Sg 89Uw== X-Gm-Message-State: AC+VfDzWQDHBCcScgcGUKxHgaXTBeFN10JO5WOSoDkaW0ndHO2KNwaeW szQfRtaEtHIlDSufEdSDlPV0CQ== X-Google-Smtp-Source: ACHHUZ5b6N+FK7PXoTwlvRKh6LsH0vyG6YXCaEWGf5NRL5zfUiNEb8MHT84h6djsafTCO/ahpimwww== X-Received: by 2002:a05:6214:764:b0:62d:e913:f9ae with SMTP id f4-20020a056214076400b0062de913f9aemr22933956qvz.1.1687500727864; Thu, 22 Jun 2023 23:12:07 -0700 (PDT) Received: from dread.disaster.area (pa49-180-13-202.pa.nsw.optusnet.com.au. [49.180.13.202]) by smtp.gmail.com with ESMTPSA id p28-20020a634f5c000000b0055387ffef10sm5712930pgl.24.2023.06.22.23.12.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Jun 2023 23:12:07 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1qCa1b-00F8aV-2d; Fri, 23 Jun 2023 16:12:03 +1000 Date: Fri, 23 Jun 2023 16:12:03 +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, linux-kernel@vger.kernel.org, linux-mm@kvack.org, intel-gfx@lists.freedesktop.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-nfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH 02/29] mm: vmscan: introduce some helpers for dynamically allocating shrinker Message-ID: References: <20230622085335.77010-1-zhengqi.arch@bytedance.com> <20230622085335.77010-3-zhengqi.arch@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230622085335.77010-3-zhengqi.arch@bytedance.com> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E4B48C000D X-Stat-Signature: ggjcau51hfhfskdikaycqzu6gp1df35j X-HE-Tag: 1687500728-214609 X-HE-Meta: U2FsdGVkX1+ld5bnK9NJU3qf29XkLfD3aoLERxvj1MIr9bgt54sZ06E6GYtVr0KK2lp3DScORRk9JTrGOBKsGibrJktt+fPHkGASyQcZ1Hjm4H/mMvCyLMoTNFLDhfYOjBlXmcgY3zrCEdmx4U9tt2ioxTRiqYrVCP79WhNs94fV5B23zHYyR4UnIdAi2DY0bC1p8fstYCRxlt/7lZqkJat1kGpCP0b/G2cBg6dUKFymajV7gQvC/7VmFMDvKWBKa/nfEE4yh/bmtsqU83rundeBxjtVyhOjA8vPMA60Asc4VgC7g/tmvRpGXrNokAlM3WrTDm2YwsCfCHn4xXJ1gFYgGMRwIHeKEPv3r/mq5rrK6IpJIdOgMc4rRcZoOtUvQ05sj4H4RfyE/bRoBIMRfg4QNWlwafhRhdxW6GG/TRnCG1AApXm5YiwICoEods0OUF5PiY1M462UP2fBHpZE8NdBySrP36Rwnv8bPSKiMHGbylTQ6inmWZv+ATi/ZKSetpYWCEP9oL+g+snGLwfX8Ac9q6L6atYkYI6Kf/mM4S22kpeIUv6kHEVlcdZx8/j4+epp77wWTnlsNJ7NWM+ELQik3T/iQWmpWzmxSlqXZCQBQ5/dznz0LsBKTVzJ+hRsS2l+8z0zFnYTDH79nSZS6oA5Wax0/OlDWiHh22JOsaaNbJtTNiNxEfcoWRP32x3wLhzVzVkHtWCR6PiYKfT56ro+p80ap1A32tdEiNM9Yns0oBW4Hba37GG3qQjpPuvtiGw6YhxD/6WjQ+r8Pz+8q+3z2RIJk5SpKB2h4tL33JwUO4oVKOl5156Z3SnDzvwL2gpu6W5Rs3dHSVgHEPYuU/KRKzuB6lZX/Dsrv3GDBVJm7VLHQDKgOXTcRiRscWZ8l23v33M9NvZ2jGzaOGIQssoV1hK7wn3hJgzdmWImS18yg0T3wcGSK14nCiz9DVrm/RY76l2QcWOAq3erWhC bA0krRMD HJ8g/fVy0+omOmqSuZCiE3JLIOyLyBSOqmmz1O9paodhqaqjyOTIYhOVlZkU3yq6gYGSzZx5pxKbVLNqTR9YWybTL4QceM8NXUAF0gwAcLga3Ur37JFnKc+4Neh0qvf17+FoFLSIUx1RFWgNyI/5bFIzYs/eFmomAdrei4PeAobM3GiF9H3GAevbCZZE+lXBlJsjh/4eYakcEAwpYBI9naYM9+bQ5Yje+nIzyRmi8dJCMM254g/DkYHhpnPaGB3KKqGon1FO+rms2sALUJh2FQBdefnNknrgeVtB54N9UPtgG4t+isW4Mz0SU2JFcX9MhQScYP4G+qBCTYfpQaxyB84ByyG3EOBDzDKfM3z7QF/dFnjdwkT84a8CJOSLiZCbq/7npXRK9XbV/AQZdsTVMmVQIXl3/SHuwYyGGf0K7mdyDjVkgNGBJ3ROCaSG6nHJH6w0+w2W2SN9dSxmuD6Tp4SxLVWj7k8y4OIxz2KyIlZCXfglSqDR2q6/zy/MuqYwSpTkI 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 Thu, Jun 22, 2023 at 04:53:08PM +0800, Qi Zheng wrote: > Introduce some helpers for dynamically allocating shrinker instance, > and their uses are as follows: > > 1. shrinker_alloc_and_init() > > Used to allocate and initialize a shrinker instance, the priv_data > parameter is used to pass the pointer of the previously embedded > structure of the shrinker instance. > > 2. shrinker_free() > > Used to free the shrinker instance when the registration of shrinker > fails. > > 3. unregister_and_free_shrinker() > > Used to unregister and free the shrinker instance, and the kfree() > will be changed to kfree_rcu() later. > > Signed-off-by: Qi Zheng > --- > include/linux/shrinker.h | 12 ++++++++++++ > mm/vmscan.c | 35 +++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h > index 43e6fcabbf51..8e9ba6fa3fcc 100644 > --- a/include/linux/shrinker.h > +++ b/include/linux/shrinker.h > @@ -107,6 +107,18 @@ extern void unregister_shrinker(struct shrinker *shrinker); > extern void free_prealloced_shrinker(struct shrinker *shrinker); > extern void synchronize_shrinkers(void); > > +typedef unsigned long (*count_objects_cb)(struct shrinker *s, > + struct shrink_control *sc); > +typedef unsigned long (*scan_objects_cb)(struct shrinker *s, > + struct shrink_control *sc); > + > +struct shrinker *shrinker_alloc_and_init(count_objects_cb count, > + scan_objects_cb scan, long batch, > + int seeks, unsigned flags, > + void *priv_data); > +void shrinker_free(struct shrinker *shrinker); > +void unregister_and_free_shrinker(struct shrinker *shrinker); Hmmmm. Not exactly how I envisioned this to be done. Ok, this will definitely work, but I don't think it is an improvement. It's certainly not what I was thinking of when I suggested dynamically allocating shrinkers. The main issue is that this doesn't simplify the API - it expands it and creates a minefield of old and new functions that have to be used in exactly the right order for the right things to happen. What I was thinking of was moving the entire shrinker setup code over to the prealloc/register_prepared() algorithm, where the setup is already separated from the activation of the shrinker. That is, we start by renaming prealloc_shrinker() to shrinker_alloc(), adding a flags field to tell it everything that it needs to alloc (i.e. the NUMA/MEMCG_AWARE flags) and having it returned a fully allocated shrinker ready to register. Initially this also contains an internal flag to say the shrinker was allocated so that unregister_shrinker() knows to free it. The caller then fills out the shrinker functions, seeks, etc. just like the do now, and then calls register_shrinker_prepared() to make the shrinker active when it wants to turn it on. When it is time to tear down the shrinker, no API needs to change. unregister_shrinker() does all the shutdown and frees all the internal memory like it does now. If the shrinker is also marked as allocated, it frees the shrinker via RCU, too. Once everything is converted to this API, we then remove register_shrinker(), rename register_shrinker_prepared() to shrinker_register(), rename unregister_shrinker to shrinker_unregister(), get rid of the internal "allocated" flag and always free the shrinker. At the end of the patchset, every shrinker should be set up in a manner like this: sb->shrinker = shrinker_alloc(SHRINKER_MEMCG_AWARE|SHRINKER_NUMA_AWARE, "sb-%s", type->name); if (!sb->shrinker) return -ENOMEM; sb->shrinker->count_objects = super_cache_count; sb->shrinker->scan_objects = super_cache_scan; sb->shrinker->batch = 1024; sb->shrinker->private = sb; ..... shrinker_register(sb->shrinker); And teardown is just a call to shrinker_unregister(sb->shrinker) as it is now. i.e. the entire shrinker regsitration API is now just three functions, down from the current four, and much simpler than the the seven functions this patch set results in... The other advantage of this is that it will break all the existing out of tree code and third party modules using the old API and will no longer work with a kernel using lockless slab shrinkers. They need to break (both at the source and binary levels) to stop bad things from happening due to using uncoverted shrinkers in the new setup. -Dave. -- Dave Chinner david@fromorbit.com