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 84D2BC48BEB for ; Fri, 16 Feb 2024 08:47:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E47808D0007; Fri, 16 Feb 2024 03:47:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DF7F48D0001; Fri, 16 Feb 2024 03:47:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C98978D0007; Fri, 16 Feb 2024 03:47:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id BB0288D0001 for ; Fri, 16 Feb 2024 03:47:48 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 52B2E120377 for ; Fri, 16 Feb 2024 08:47:48 +0000 (UTC) X-FDA: 81797039016.13.4B0CB42 Received: from mail-oi1-f169.google.com (mail-oi1-f169.google.com [209.85.167.169]) by imf09.hostedemail.com (Postfix) with ESMTP id 796A0140015 for ; Fri, 16 Feb 2024 08:47:45 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=JZ4geO8a; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf09.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.167.169 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708073266; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=upnkKGhfzc8qqiqM2ipvrycRNTREZNhhxEHc3oR8BjY=; b=YqOLLhKgPKSfuO5ZwUK6C9EGMdsRrgf5sBZg7xiwY9/pEbQ6xVbFyPwou7Oy/FIQHXOgNt 3sZBzfysGR/dSPVDc4iMghzz1qkJc/dFaqMoo4IVokcCtQG5QlovikmQAlwfVbGbPZjLvG 95/5vUetW0FX4rigLiXVE2Vhrvi31MQ= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=JZ4geO8a; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf09.hostedemail.com: domain of zhouchengming@bytedance.com designates 209.85.167.169 as permitted sender) smtp.mailfrom=zhouchengming@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708073266; a=rsa-sha256; cv=none; b=lk96Idhr1cMHokBtqU02zMkhpxT70st7rqbS3hb8eH0wy0jSgA5e4z2zM0lrmAzRPM8YhB aVdSnEeU7sosqXNwWW4hFA2C1gWdVg165YvklPBzBmYG4EDX62iNQ8K2S7OMVvGPDKp8Y4 nVEX/T+uSXs3hFR2p9jz2oAoQvqikiI= Received: by mail-oi1-f169.google.com with SMTP id 5614622812f47-3c135913200so755333b6e.1 for ; Fri, 16 Feb 2024 00:47:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1708073264; x=1708678064; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=upnkKGhfzc8qqiqM2ipvrycRNTREZNhhxEHc3oR8BjY=; b=JZ4geO8a4gWnRAdDXdrKh2b5NbBvyKvFulQJyeGYe5H1OTGECsramXfEOJ2Bf3Nyu7 jJjuU4K/tbsKkkoewSZamfAv0Hfp9tk5wadzkFS4LbXAXwqr/DmImKZfKYkG0dxRYuLI P8Xw+TpNUguZzNDZkFVJk3a32DAk6pjiID4sIlcVsdHGIPWjjot6C18DyGljXj3EaHoA cqcxKcE73KyD8Y59dJuVQfqRj2jxboTSaYVFDdE71U3iW86/x7m4bO3ukvdMaDOU6y5H Wi/tH/zy6g8DFxzFniJXKlQnLSPUSTnYUE0WM0fvhb7s6P33lVqC/zpfFMwM/6VFA3nl NlYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708073264; x=1708678064; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=upnkKGhfzc8qqiqM2ipvrycRNTREZNhhxEHc3oR8BjY=; b=aGW5+ODEk/RwaeFpZzwxubToOmFKL3WwaY2V3XoLpRWjWooC7LUg+3neZ1weV7yBEC SixkOtG8XTy2px9C7PXQh8Sx3H1W575EjyHDnFGJvgSXQUOUjjtPekyGYvdbpsnx2Wlg 8Lf56BaXvXSbPw4N0IgOj/8NprBRLa577+nO422qmwiMovExJNmhZlNtc2HrCLHQsEw5 JD8GKjNfXeOX3k/fk8PsL3HCi01pdXlIIP73WUGQ1eRPUPLKrbyEn+eQ4ar4wUji+V/P NHhqEQ5YbS3a9cDnj76DUJmb3rTVyz5PEcRzzRVp/xab8hTdmOk41o2XdQFegofQhN1J fCXA== X-Forwarded-Encrypted: i=1; AJvYcCUleNccaIJxVFrw3jG8660yEBMCrBBhqtvH5LkFtQdgmhLVetUn2sulmlLeqkZ/Qq1aFvhBcJUlYEI5/1kPkeTtZOE= X-Gm-Message-State: AOJu0YyQMfKh1ASNobT4BYJjJO3GD6YacjTn6Haj+r9dn2BIwqEjivTh jb67BbZLCSlCRG5czeB/009S85uD5lITyDhWCVpgOHyfUiEdsHAiAdvIgK1uHSE= X-Google-Smtp-Source: AGHT+IEqstXlPoKBOibfxM+Y85KLlPqpY851hu7oh3SjO27Hz6LOHiAT6sfWa+W7yLi0PA9vPrimoA== X-Received: by 2002:a05:6808:1448:b0:3c0:4129:946b with SMTP id x8-20020a056808144800b003c04129946bmr5317425oiv.46.1708073264459; Fri, 16 Feb 2024 00:47:44 -0800 (PST) Received: from [10.4.195.175] ([139.177.225.229]) by smtp.gmail.com with ESMTPSA id r3-20020aa79883000000b006e042b50c39sm2666275pfl.144.2024.02.16.00.47.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 16 Feb 2024 00:47:44 -0800 (PST) Message-ID: Date: Fri, 16 Feb 2024 16:47:38 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/2] mm/zswap: global lru and shrinker shared by all zswap_pools Content-Language: en-US To: Yosry Ahmed Cc: Johannes Weiner , Andrew Morton , Nhat Pham , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240210-zswap-global-lru-v2-0-fbee3b11a62e@bytedance.com> <20240210-zswap-global-lru-v2-1-fbee3b11a62e@bytedance.com> From: Chengming Zhou In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 796A0140015 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: u61b7jzc6yj68u9r16d6mrr6eckhyjsq X-HE-Tag: 1708073265-368400 X-HE-Meta: U2FsdGVkX1+WrMZN8SBjrgSKK79rOO0mbL6doQM3tc6fIBod1Du4HWZDi64ZqO1GOamim7OVNNNkggah8PBeE7yifGXcqUObPCXPEDjtmtjI5rQ7tTw/UfKtsp8wAqmfkh8hpyl5njUbRLLvDAYwURyaqqAbPm2FhIZk5eBfSqL65exgjr8+z+h/pu3Trfp5N//8fHaLL/D8DftNJi2BcATIB/hR8mXCrrk/fYgjGs5vGfEJRA3BSF9Vh22pAfRaXesmHFe2CoQF7OvYLnwCR/50FlJVoq1eAfnIB5F8louJa4tOOP7E50/+K3A/emc1bbTVubtKnR/rW1IpZIPOLqOk91CdgYNR7rGtLI7VAu9nhU7PLfbP3WltNf6v1zBPhO2V7WMCisaL+pVDWH2mghYgUjHkth8Sm9H4SjAcyBcpGqpRjI30FdaA6kM92gx9ed+LJi77G0sqhUvgMwpHg3PzKerIuJlYzFDlSSoM3fFaRH7vF/Wk0BmiLb0QHuwZitxkNNdGAR/odhUq8IVWWtLTXanc8au1W3MLvexkRnaEMy2nAjLPCrn2mHRyeMbjCXNtSgZk7ZJrFq6fjyAYxusx0DAIusRCOCmOoCJVO2fnDKOgpmXv8HjwxitNxkmhvyeGqcoIJjmM1CMGodkNY2NomE2OKAz2HKngQ+WHMHKkaJ2wlH/Eei0yCbbIVVNgo1LKmP7p6vkBT/Tx7ARckHFpnnYa0OjAPM6Y4PbBZG9pyDu3JwD5bgG6dffZ1qNtXg8xt5DemzNZPsxCoCs02KEBhBPLsn1bjDThjHAkaoaeSGZGP8dcUBvAvmcEO7IeioR53eOkC2tOMxAMT7/V2eXVzfvmCSeba3ba9z0H+s7X/mPgTR4kpgSffLZ0/i4fZYtnyrFQxL4PnCO+kUEKUs4PMbOw3xX/LBTNZkvWOtgwWy2PiyeomceYbNVS8UQBgvmI3E7vdX9hTQk242U HezLc9ZY qV1UZceJzy71aSl4C92/l/0Z4b7rbDRJDIIs//tcsM425M08qluY4sMVgEa2V6FlH0SElYXsJTYDT5Rljb98bwr3LTDPc1x30ZdhfaEQ38iZJl9GhaWMcX/fh/H7YYdiJLjO2e+Ou3IVoRGU17DrA2MVRqs5qW6kygW9clEziUf3JUIrDuYMrY8ygDVK8QyPl175B6wDuVuvzQfV8N6QxtENnfSHUJyf9F/7VusNqcNDYi+Na9AU4fVFvEgiiLk9zwcus+DvbIwCVql1uzgNERdcmQqIqw4L7/bxL0eLuMunx/5/HPLI0T12DTn51t6R+svAOuk/qDqEHOBq+PcQxtXxN49/hoTApWwPh4JeTWs7n34t+6IqNPa3biCekw5aJx+aV6gdohc8UhCWqV/WybwG7UB42LDllv7vb6KJftsTBpmMHHsBrztdLRFMIDBKgUJ0KkcRFmTEdN99QiVP0B190urwHJKEgp3LJWBnBQOgENP0S8nrbrDt3VyxQB+rVQiuVW3hrmdLiOnNJC8Q5nrc8Cg== 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: List-Subscribe: List-Unsubscribe: On 2024/2/15 03:20, Yosry Ahmed wrote: > On Wed, Feb 14, 2024 at 08:54:37AM +0000, Chengming Zhou wrote: >> Dynamic zswap_pool creation may create/reuse to have multiple >> zswap_pools in a list, only the first will be current used. >> >> Each zswap_pool has its own lru and shrinker, which is not >> necessary and has its problem: >> >> 1. When memory has pressure, all shrinker of zswap_pools will >> try to shrink its own lru, there is no order between them. >> >> 2. When zswap limit hit, only the last zswap_pool's shrink_work >> will try to shrink its lru, which is inefficient. > > I think the rationale here was to try and empty the old pool first so > that we can completely drop it. However, since we only support exclusive > loads now, the LRU ordering should be entirely decided by the order of > stores, so I think the oldest entries on the LRU will naturally be the > from the oldest pool, right? > > Probably worth stating this. Right, will add your this part in the commit message. > >> >> Anyway, having a global lru and shrinker shared by all zswap_pools >> is better and efficient. >> >> Signed-off-by: Chengming Zhou > > LGTM with a few comments, with those: > Acked-by: Yosry Ahmed > >> --- >> mm/zswap.c | 170 +++++++++++++++++++++++-------------------------------------- >> 1 file changed, 65 insertions(+), 105 deletions(-) >> >> diff --git a/mm/zswap.c b/mm/zswap.c >> index 62fe307521c9..dbff67d7e1c7 100644 >> --- a/mm/zswap.c >> +++ b/mm/zswap.c >> @@ -176,14 +176,18 @@ struct zswap_pool { >> struct kref kref; >> struct list_head list; >> struct work_struct release_work; >> - struct work_struct shrink_work; >> struct hlist_node node; >> char tfm_name[CRYPTO_MAX_ALG_NAME]; >> +}; >> + >> +static struct { >> struct list_lru list_lru; >> - struct mem_cgroup *next_shrink; >> - struct shrinker *shrinker; >> atomic_t nr_stored; >> -}; >> + struct shrinker *shrinker; >> + struct work_struct shrink_work; >> + struct mem_cgroup *next_shrink; >> + spinlock_t shrink_lock; > > The lock is exclusively protecting next_shrink, right? Perhaps we should > rename it to next_shrink_lock or at least document this. Ok, I will add a comment to it. > >> +} zswap; >> >> /* >> * struct zswap_entry >> @@ -301,9 +305,6 @@ static void zswap_update_total_size(void) >> * pool functions >> **********************************/ >> >> -static void zswap_alloc_shrinker(struct zswap_pool *pool); >> -static void shrink_worker(struct work_struct *w); >> - >> static struct zswap_pool *zswap_pool_create(char *type, char *compressor) >> { >> int i; >> @@ -353,30 +354,16 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor) >> if (ret) >> goto error; >> >> - zswap_alloc_shrinker(pool); >> - if (!pool->shrinker) >> - goto error; >> - >> - pr_debug("using %s compressor\n", pool->tfm_name); > > nit: the next patch introduces a new failure case between this debug > print and zswap_pool_debug() below, so it will become possible again > that we get one and not the other. Not a big deal though. Yes, not a big deal. Thanks!