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 9AD6DC7618D for ; Thu, 6 Apr 2023 14:53:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 392AA6B0071; Thu, 6 Apr 2023 10:53:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 31C386B0074; Thu, 6 Apr 2023 10:53:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E3816B0075; Thu, 6 Apr 2023 10:53:53 -0400 (EDT) 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 0BBCE6B0071 for ; Thu, 6 Apr 2023 10:53:53 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id D26E2A0F36 for ; Thu, 6 Apr 2023 14:53:52 +0000 (UTC) X-FDA: 80651260704.01.ED47689 Received: from verein.lst.de (verein.lst.de [213.95.11.211]) by imf12.hostedemail.com (Postfix) with ESMTP id 1C59340014 for ; Thu, 6 Apr 2023 14:53:50 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of hch@lst.de has no SPF policy when checking 213.95.11.211) smtp.mailfrom=hch@lst.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680792831; 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; bh=ssQN+dBZoakVC4TPQlTiHUnLUWWrhVH3l7VrAjYLSMc=; b=WdynH28sArjKHDJZTQKmuHpt/WCJwsujpkMi8dRA3T5yQHYf4c//uammp3P63f2ajvrArp 1VINb9hn7yFpskYJV8TtP1+y+aKTregoKpzzC/efawG0Iv/F7/zJN9WoXSXswFXJER754w OWRZ/v2hH7Dm8mfyJZbT8rxWEAoXidQ= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; dmarc=none; spf=none (imf12.hostedemail.com: domain of hch@lst.de has no SPF policy when checking 213.95.11.211) smtp.mailfrom=hch@lst.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680792831; a=rsa-sha256; cv=none; b=HlJikEGCttC4xb0po1r0h4AIBV3cjlRpe8wOw+vbb+PRvT4fovp8tmF+k9NJRxyucLPvRo sq0ot8IitoOMRXDEkwdkT2oHolxJX6BRgaPmPx1a982xtkQmV0swNVDYt8oOH/RD2is0rz Xedz1AklC1+Z6BoKeqaXShwz6YijgEY= Received: by verein.lst.de (Postfix, from userid 2407) id 547E367373; Thu, 6 Apr 2023 16:53:47 +0200 (CEST) Date: Thu, 6 Apr 2023 16:53:47 +0200 From: Christoph Hellwig To: Liu Shixin Cc: Seth Jennings , Dan Streetman , Vitaly Wool , Andrew Morton , Nathan Chancellor , Christoph Hellwig , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH -next v8 3/3] mm/zswap: delay the initialization of zswap Message-ID: <20230406145347.GB11839@lst.de> References: <20230403121318.1876082-1-liushixin2@huawei.com> <20230403121318.1876082-4-liushixin2@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230403121318.1876082-4-liushixin2@huawei.com> User-Agent: Mutt/1.5.17 (2007-11-01) X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1C59340014 X-Stat-Signature: zx1b6m8tztf41zpcw1zz339pcsxseerq X-HE-Tag: 1680792830-82932 X-HE-Meta: U2FsdGVkX19Mq6n9ovt0iuHz2ADB/2A3R4bC0O6+OdnjWhzVTJS+Zc6v41NRKavRIhR2CkBchc6x/5XnOizIIjO39zwYZ8wjy2PbdgBZVwmA4U9b8+m3BxivhGpCUcaBZrFNda+lpufbWFJG9evGTlabsBdt+TwK5AAYVQ7aCUIpPJMUjMs2ZODvZtY6t7Oc5lLJYXlO6c4yRRK/qNenyVuxxF+Gh312XrkX7vh/wiwyj1rvtgGjXV1AfQBe6gZ5/KTDyHYIvBN4FFhtHoRRLhmAQZOfs6ohRxSZh2QrC1KBBHOaVzDZzbDLWIcQaA4M3LQfxAV+WrfRu+JNFA/tBhyMScFeKcHuBjpmYaz7qjIXupGrN8mkiOcVC6fB8j00g/CFbYVHR0zU1VSV0p50QkdSLVZtaPiKlk2RVCSOb7OX4bYKJwGzQPRzcDRgHaObP+x4jCwEvoVfms1uPgI7CBteqLWA7KiMkCRsnkeEHIlRlp0uZb+OaaUx8qwaBjBE6WiG1SGXp8HUCm6rpTIO4VLHWj6yR1TTxBeczVYpRyqE6lLCyEdlSID+ID6lSjmuAmgUgbuAsag17YRQih+y5Q4FNtdj8ZTYKFtUNwFqtS4ft4czfogqvoH/KNb9TfHbhKaX51OaoWKzdBKrYNLDdTV34Zxb1OIdXRm29Vu3F/8Vdaneqo5N5DQX3IYs5DC3OQ96KVK/AhJEuYpASBqvmbbGkbmRUwRM0SSsaL43IaWDHD/JOZ0ktu3VjsDnZVTUXSnMlS9ZzWKLlq7reA/qS+EfP9dqfIfY2mRgL84CwhZIPfXTLVQ9CchXezbMm18EQvHC8Qq6dUH2XV7c3HQzzcHZO+Qm2YiywKw7VIZAKRNp0YV5XjtjekE624VI/2kWxFh1soNFuMI65KYDWrtG2E1tKDnfMwWLTcGxtGp+leFj3m/2f+5qeah7N1DIqqU3aPuZplc/YDuiNoXEOpf nLLuSc8i 7bIGn0fLVum41vIhjGqBhSR9hbukV7QBZgZwJotJPsCy7rQNe3IAV4pQqVrFlnxuODeLTqtJHdXT4kyyMFrJdbCKSRTFGu7nxUuuS 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, Apr 03, 2023 at 08:13:18PM +0800, Liu Shixin wrote: > Since some users may not use zswap, the zswap_pool is wasted. Save memory > by delaying the initialization of zswap until enabled. > > Signed-off-by: Liu Shixin > --- > mm/zswap.c | 56 +++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 43 insertions(+), 13 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 9169c2baee87..14db57450bfd 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -81,6 +81,8 @@ static bool zswap_pool_reached_full; > > #define ZSWAP_PARAM_UNSET "" > > +static int zswap_setup(void); > + > /* Enable/disable zswap */ > static bool zswap_enabled = IS_ENABLED(CONFIG_ZSWAP_DEFAULT_ON); > static int zswap_enabled_param_set(const char *, > @@ -222,6 +224,9 @@ enum zswap_init_type { > > static enum zswap_init_type zswap_init_state; > > +/* used to ensure the integrity of initialization */ > +static DEFINE_MUTEX(zswap_init_lock); > + > /* init completed, but couldn't create the initial pool */ > static bool zswap_has_pool; > > @@ -654,7 +659,7 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor) > return NULL; > } > > -static __init struct zswap_pool *__zswap_pool_create_fallback(void) > +static struct zswap_pool *__zswap_pool_create_fallback(void) > { > bool has_comp, has_zpool; > > @@ -763,21 +768,28 @@ static int __zswap_param_set(const char *val, const struct kernel_param *kp, > char *s = strstrip((char *)val); > int ret; > > + mutex_lock(&zswap_init_lock); > switch (zswap_init_state) { > case ZSWAP_UNINIT: > /* if this is load-time (pre-init) param setting, > * don't create a pool; that's done during init. > */ > - return param_set_charp(s, kp); > + ret = param_set_charp(s, kp); > + mutex_unlock(&zswap_init_lock); > + return ret; > case ZSWAP_INIT_SUCCEED: > /* no change required */ > - if (!strcmp(s, *(char **)kp->arg) && zswap_has_pool) > + if (!strcmp(s, *(char **)kp->arg) && zswap_has_pool) { > + mutex_unlock(&zswap_init_lock); > return 0; > + } > break; > case ZSWAP_INIT_FAILED: > pr_err("can't set param, initialization failed\n"); > + mutex_unlock(&zswap_init_lock); > return -ENODEV; > } > + mutex_unlock(&zswap_init_lock); The pattern here looks a bit odd. Can you split the code that the ZSWAP_INIT_SUCCEED case falls through into a helper, then just assign ret inside the switch statement, break out and do a single unlock and return? > + /*if this is load-time (pre-init) param setting, only set param.*/ missing space after the initial and before the last * here. But except for these nitpicks this version looks good to me now.