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 9E8E6CFA45C for ; Wed, 23 Oct 2024 18:12:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40DCD6B0092; Wed, 23 Oct 2024 14:12:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 396856B0096; Wed, 23 Oct 2024 14:12:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 210A26B0098; Wed, 23 Oct 2024 14:12:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id F3B3E6B0092 for ; Wed, 23 Oct 2024 14:12:53 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3AFCE140DD5 for ; Wed, 23 Oct 2024 18:12:35 +0000 (UTC) X-FDA: 82705661892.20.08DF98B Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf21.hostedemail.com (Postfix) with ESMTP id 518E41C0018 for ; Wed, 23 Oct 2024 18:12:18 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=P1I5yv72; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729707047; a=rsa-sha256; cv=none; b=exsT/Vb2fcgZizHNoKqXA74YNNgkokO4CNZyjfgYidgxz1cDUwZxU9XT5i1RMtfLbiQU8g sN7xzLh55H+gZR57QQJPn62OrwiD0vAVY8Ey1s3URjQaYB4iINzpH01rvT4d1ExcN+vCDK +tkq0YrWOXXcvIvA2jCGl3LgiaX/POc= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=P1I5yv72; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729707047; 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=LDi5VPzX6c61t1Xhrb5vlpguCbYE1+CPmZ5jAWd7OIw=; b=FHIAiZ3yEaVFvYo7RJq0RNdysXEEeIvLlcGBt1pBEDBq71DIs3hTd/TS9+7KWZd/kwOd/F 7oGnRW3GTQux9zn5bHBDKnW4//1HM2N8aaODSZprzS00Jf7GlOW6Ljis9aQDW+SMTuTNty 6kX2ijYIVmG+z0kcuxVNZk1cH8rV5JU= Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-5cb6ca2a776so90945a12.0 for ; Wed, 23 Oct 2024 11:12:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729707170; x=1730311970; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=LDi5VPzX6c61t1Xhrb5vlpguCbYE1+CPmZ5jAWd7OIw=; b=P1I5yv72Mi4L5a0J9Z291iZoyvcLeZSgh3LhwJf33FWUM5lt/hHdEipUErVd14uL1E rKAkvcHLkIVjIXaSp1efhAqVWLAxsG24sLNmZJuKSDtH+NjMpPiT6hllIjH2TFtfZHho ppNjcTw2umpqct/iRdEkcLpg6m1Tc+0KqOfcyPeq9k2aaMj94TIoj/YpFdF84lyFc+4y S0MhXC2sqWz7LPr1VNQunkpsM5jlW39tQ4K8+GvD2QY1W12wn0mmPHp8T9u79/WWG3lc q1vweh7WQmfpRdpgnkHC5kLexzHpNUjFNYPqu1dLShRwMJHgm6lCsWqq2rbqgnD+iroo inMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729707170; x=1730311970; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=LDi5VPzX6c61t1Xhrb5vlpguCbYE1+CPmZ5jAWd7OIw=; b=SdLz79y69ZCdJpYX8V5Cf5Mj4j8y8NuSJpNC0YPj8XuyxoNWydDToSM6Kbd5ZNd97R 7/wiqs7pVVhVg1iNiXb7c/exkFq90tH43349xArJZJTLNwuYHq8Wuft4HU26Y+weaa+y L/R+aYw1yiSQvlS3v+R1Xn23QK6/JWNTCc9xA8r26SEbhUNYSCtubxAoNSEyc6dViuyy SooZ3x0OgX7aS3NOnve1bCWvcc+BgtaDO0SvIvSFxmaaqlJWiejviXzQIlr6DyrN4fsd qUQ9UxR7F4PwZPQRAgh/n0vTbmkLNhJweFznGOKu0xxEez5nXp3bq4s2LZhKbMJ4piO/ jXbg== X-Forwarded-Encrypted: i=1; AJvYcCUuo8U52yYnvDbDAKS/6f7B1fS7iTLNrUkvvyjARdraEcX3SSWN3GeNIgDWP40yo7XmhXx50tZiWA==@kvack.org X-Gm-Message-State: AOJu0YyTSg650r18c/iDVAixbaVp7ek1n9FoYN7lAGe3xy9e2NNDa/ll ZI9sWi1aHYIiLI61Z9EkPYRPcRxctpeqdrFZkEXNV62zdOcuJyDhJGlVpiO3LMOe4bakTllDRBE EYv/4PXR6lUXzfJbgIzlsOp7ucLWnJFAhpbG2tc+823ggqjSa3Q== X-Google-Smtp-Source: AGHT+IEWtGBqmChvvFUDCvgxebL4MPsU63ShJMOn68yqj4BjRtIC6qhUzt6L4WKNcUU9vsQpXuTBTyq/d29UkOioP7A= X-Received: by 2002:a17:907:3e9e:b0:a9a:dac:2ab9 with SMTP id a640c23a62f3a-a9abf92dd94mr340448966b.42.1729707170109; Wed, 23 Oct 2024 11:12:50 -0700 (PDT) MIME-Version: 1.0 References: <20241018064101.336232-1-kanchana.p.sridhar@intel.com> <20241018064101.336232-10-kanchana.p.sridhar@intel.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 23 Oct 2024 11:12:12 -0700 Message-ID: Subject: Re: [RFC PATCH v1 09/13] mm: zswap: Config variable to enable compress batching in zswap_store(). To: "Sridhar, Kanchana P" Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "hannes@cmpxchg.org" , "nphamcs@gmail.com" , "chengming.zhou@linux.dev" , "usamaarif642@gmail.com" , "ryan.roberts@arm.com" , "Huang, Ying" , "21cnbao@gmail.com" <21cnbao@gmail.com>, "akpm@linux-foundation.org" , "linux-crypto@vger.kernel.org" , "herbert@gondor.apana.org.au" , "davem@davemloft.net" , "clabbe@baylibre.com" , "ardb@kernel.org" , "ebiggers@google.com" , "surenb@google.com" , "Accardi, Kristen C" , "zanussi@kernel.org" , "viro@zeniv.linux.org.uk" , "brauner@kernel.org" , "jack@suse.cz" , "mcgrof@kernel.org" , "kees@kernel.org" , "joel.granados@kernel.org" , "bfoster@redhat.com" , "willy@infradead.org" , "linux-fsdevel@vger.kernel.org" , "Feghali, Wajdi K" , "Gopal, Vinodh" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 518E41C0018 X-Stat-Signature: 8rirfxw8bitp5c1e8fkqtzg6bem5px35 X-Rspam-User: X-HE-Tag: 1729707138-140765 X-HE-Meta: U2FsdGVkX1+v8NItn9LwVLslCjtI5L4zkjR9xpjQZhQ9srWGAMweh9gI3SsMACc71awAUZqI7rqWlQm4KUng8+U0ddYasI2fr58DHC6H4C1wIZ5BnVZKP1sanBZ4tW1vKII3OOW684343qQkBbAyn76qxLlHUMgJRyPxwsnpZU+mlAh8G7Mg5dxybLAVL2oK+T0cdpsnvoc38O+TfoT8xB0kcUI04MbSICmy+9YDGfFl8mfbQUpRHoJMiC0Jk9KWFnKCyXvC4FQYt6/oPNmZkEBXl7pgc25zK9CBul9/YHa9liJO6OhLeqd7QFcd6sBUGrCHu7CMlmO5aj9Fv6u6Kouu3PcgO1HWF65JG4xAbyR/3+5y4y++Mznz4zRCJSFdyYv7p96sYFW49v02K0MPF526lSq0r0/5ZzcDhLgJewdN3lvlHq6BPIg2ALmWDzDqz7oYVeAGE+w7BRxzWrOLG1Md3m7wsu6Qb1IXnv93vYAVgf/2piNEGtpCfRVt3yfTdcQ3DjbCIu2xTRVlhUgmn4EQIPWnLMASlR03stfzG1h2oGI3H48K51DSRaeJPQ2H84r/3Qdli1wY+Zxp9rB481p6Il8zMuPJNTEnYckYxuKx24hHMH+Y8/7wUNBiheHgXNik/JHSUQu0o6HEJ+iRF5UWdAsRDftzTNE0FmBrll0c0Y2gw5yVTYi/lSE2GFSL13oPlT1sot0pawEc56NbRN0y5rKr60GCzQr4bcyrCCnZpenkdk+m+IeCta06fKvNUNAmXrSUs8IlRGQCy2F7jfxzLHMKJ1l2bcU71HYACt3/GUI7MWAucj2ic6O4tRwXyrhjcOUwk/MyLTMwISaJ19X6Qa0mHfiy7dBoomp8HsAzmNXsQunm1JGoysfdU6EuPOhfS9/xcGYreDb+72ZLU7uYD8qSni3QhTKNOSc7rnE+ANLX4382gFpUeIB5VcxkwZ5QpAilKd2uUgKyA/R 6rZMCgy/ EMqJ3pE+ZJLnBJqwzhfcZHmbfiW8rG8teCleA6WTtOzLpMTHXUMFJH6U/T0khZawrNrbD3ZBrOmOEP1pV6DDTfG6z1+/fnovwUj18l0ZGTscIf9LbDum5iDfz8lOAXq0siTMxGIhsEawyzONzbks9WCFnWy3aDYmYiyk0D7ruvy8jLY9YAl974cf6QU4IqfdqLVOZc02S9xl7t4h/SP7liy1jO1d3oHmYsDjhpoEZ2IIYKiAs5jwm648UzM6FCtV9bzoTERySxaUaYmWfoJDkL28h4Jg+iYf9iIZvGBwFTtrLZU3J2YgynUwrXyuLF8MxJisIEvrUbx11xLT0Y6nHEDtlhNiQE/KWaKCAmKVQmgKdqo1O4ITdSin5E+s2I1x2zgeqIeLJA+/Dis7Upi8hovHKXFlibDvhno1AT7qiH4Xs4lT5X84u8icFMk9Oh/7yDKz9N7aG5n1J7tubH6KQ/ALjEOoq5v0gIxiH0nRTmnwfohKzIzC5IuacLEq1Qqrs5Zdfs2rRhA1F4vsww/ON5ilagSR7dU5D1J02Yn/wUW36LDWI3V+pke7MSxMKnJVSfrriV9Z7H522gSQUlYPKkZ1xiDO8cYtObBCldo1z5W1bmZsY+h+sDTXe4fgo4OV/gBDCrYF3Cv6yTustJ8pqPROQaVtzs7KFa33hR3rWnH/1d1sK8OoHvwCQzUptdR8aYaRdq8Hae6mTT1ybZYXy80Bxr48YH/w8i8WmLYt8Gc6jKpS9dgzbr5TnH05UDCPPtBtmb34heKyD5rVtuKSCsrQaaZfLqG7HKAhEfv2qBJwcsASyBWuZh6t85pPF7FZfDjNrfvct3EsjIZcQhBvsbrZGDCPl1c1fWTyDwBvJ6dGxZQNpyLubPHtun5gd5H2dUuAKlwwv5K8jpxUOIAzWPTxo96v51WVhIRcDLfpZ3+bG4KSWZvQVj/UZsQ== 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 Tue, Oct 22, 2024 at 7:17=E2=80=AFPM Sridhar, Kanchana P wrote: > > > > -----Original Message----- > > From: Yosry Ahmed > > Sent: Tuesday, October 22, 2024 5:50 PM > > To: Sridhar, Kanchana P > > Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org; > > hannes@cmpxchg.org; nphamcs@gmail.com; chengming.zhou@linux.dev; > > usamaarif642@gmail.com; ryan.roberts@arm.com; Huang, Ying > > ; 21cnbao@gmail.com; akpm@linux-foundation.org; > > linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au; > > davem@davemloft.net; clabbe@baylibre.com; ardb@kernel.org; > > ebiggers@google.com; surenb@google.com; Accardi, Kristen C > > ; zanussi@kernel.org; viro@zeniv.linux.org= .uk; > > brauner@kernel.org; jack@suse.cz; mcgrof@kernel.org; kees@kernel.org; > > joel.granados@kernel.org; bfoster@redhat.com; willy@infradead.org; linu= x- > > fsdevel@vger.kernel.org; Feghali, Wajdi K ; = Gopal, > > Vinodh > > Subject: Re: [RFC PATCH v1 09/13] mm: zswap: Config variable to enable > > compress batching in zswap_store(). > > > > On Thu, Oct 17, 2024 at 11:41=E2=80=AFPM Kanchana P Sridhar > > wrote: > > > > > > Add a new zswap config variable that controls whether zswap_store() w= ill > > > compress a batch of pages, for instance, the pages in a large folio: > > > > > > CONFIG_ZSWAP_STORE_BATCHING_ENABLED > > > > > > The existing CONFIG_CRYPTO_DEV_IAA_CRYPTO variable added in commit > > > ea7a5cbb4369 ("crypto: iaa - Add Intel IAA Compression Accelerator cr= ypto > > > driver core") is used to detect if the system has the Intel Analytics > > > Accelerator (IAA), and the iaa_crypto module is available. If so, the > > > kernel build will prompt for CONFIG_ZSWAP_STORE_BATCHING_ENABLED. > > Hence, > > > users have the ability to set > > CONFIG_ZSWAP_STORE_BATCHING_ENABLED=3D"y" only > > > on systems that have Intel IAA. > > > > > > If CONFIG_ZSWAP_STORE_BATCHING_ENABLED is enabled, and IAA is > > configured > > > as the zswap compressor, zswap_store() will process the pages in a la= rge > > > folio in batches, i.e., multiple pages at a time. Pages in a batch wi= ll be > > > compressed in parallel in hardware, then stored. On systems without I= ntel > > > IAA and/or if zswap uses software compressors, pages in the batch wil= l be > > > compressed sequentially and stored. > > > > > > The patch also implements a zswap API that returns the status of this > > > config variable. > > > > If we are compressing a large folio and batching is an option, is not > > batching ever the correct thing to do? Why is the config option > > needed? > > Thanks Yosry, for the code review comments! This is a good point. The mai= n > consideration here was not to impact software compressors run on non-Inte= l > platforms, and only incur the memory footprint cost of multiple > acomp_req/buffers in "struct crypto_acomp_ctx" if there is IAA to reduce > latency with parallel compressions. > > If the memory footprint cost if acceptable, there is no reason not to do > batching, even if compressions are sequential. We could amortize cost > of the cgroup charging/objcg/stats updates. Hmm yeah based on the next patch it seems like we allocate 7 extra buffers, each sized 2 * PAGE_SIZE, percpu. That's 56KB percpu (with 4K page size), which is non-trivial. Making it a config option seems to be inconvenient though. Users have to sign up for the memory overhead if some of them won't use IAA batching, or disable batching all together. I would assume this would be especially annoying for distros, but also for anyone who wants to experiment with IAA batching. The first thing that comes to mind is making this a boot option. But I think we can make it even more convenient and support enabling it at runtime. We just need to allocate the additional buffers the first time batching is enabled. This shouldn't be too complicated, we have an array of buffers on each CPU but we only allocate the first one initially (unless batching is enabled at boot). When batching is enabled, we can allocate the remaining buffers. The only shortcoming of this approach is that if we enable batching then disable it, we can't free the buffers without significant complexity, but I think that should be fine. I don't see this being a common pattern. WDYT? > > Thanks, > Kanchana > > > > > > > > > Suggested-by: Ying Huang > > > Signed-off-by: Kanchana P Sridhar > > > --- > > > include/linux/zswap.h | 6 ++++++ > > > mm/Kconfig | 12 ++++++++++++ > > > mm/zswap.c | 14 ++++++++++++++ > > > 3 files changed, 32 insertions(+) > > > > > > diff --git a/include/linux/zswap.h b/include/linux/zswap.h > > > index d961ead91bf1..74ad2a24b309 100644 > > > --- a/include/linux/zswap.h > > > +++ b/include/linux/zswap.h > > > @@ -24,6 +24,7 @@ struct zswap_lruvec_state { > > > atomic_long_t nr_disk_swapins; > > > }; > > > > > > +bool zswap_store_batching_enabled(void); > > > unsigned long zswap_total_pages(void); > > > bool zswap_store(struct folio *folio); > > > bool zswap_load(struct folio *folio); > > > @@ -39,6 +40,11 @@ bool zswap_never_enabled(void); > > > > > > struct zswap_lruvec_state {}; > > > > > > +static inline bool zswap_store_batching_enabled(void) > > > +{ > > > + return false; > > > +} > > > + > > > static inline bool zswap_store(struct folio *folio) > > > { > > > return false; > > > diff --git a/mm/Kconfig b/mm/Kconfig > > > index 33fa51d608dc..26d1a5cee471 100644 > > > --- a/mm/Kconfig > > > +++ b/mm/Kconfig > > > @@ -125,6 +125,18 @@ config ZSWAP_COMPRESSOR_DEFAULT > > > default "zstd" if ZSWAP_COMPRESSOR_DEFAULT_ZSTD > > > default "" > > > > > > +config ZSWAP_STORE_BATCHING_ENABLED > > > + bool "Batching of zswap stores with Intel IAA" > > > + depends on ZSWAP && CRYPTO_DEV_IAA_CRYPTO > > > + default n > > > + help > > > + Enables zswap_store to swapout large folios in batches of 8 p= ages, > > > + rather than a page at a time, if the system has Intel IAA for= hardware > > > + acceleration of compressions. If IAA is configured as the zsw= ap > > > + compressor, this will parallelize batch compression of upto 8= pages > > > + in the folio in hardware, thereby improving large folio compr= ession > > > + throughput and reducing swapout latency. > > > + > > > choice > > > prompt "Default allocator" > > > depends on ZSWAP > > > diff --git a/mm/zswap.c b/mm/zswap.c > > > index 948c9745ee57..4893302d8c34 100644 > > > --- a/mm/zswap.c > > > +++ b/mm/zswap.c > > > @@ -127,6 +127,15 @@ static bool zswap_shrinker_enabled =3D > > IS_ENABLED( > > > CONFIG_ZSWAP_SHRINKER_DEFAULT_ON); > > > module_param_named(shrinker_enabled, zswap_shrinker_enabled, bool, > > 0644); > > > > > > +/* > > > + * Enable/disable batching of compressions if zswap_store is called = with a > > > + * large folio. If enabled, and if IAA is the zswap compressor, page= s are > > > + * compressed in parallel in batches of say, 8 pages. > > > + * If not, every page is compressed sequentially. > > > + */ > > > +static bool __zswap_store_batching_enabled =3D IS_ENABLED( > > > + CONFIG_ZSWAP_STORE_BATCHING_ENABLED); > > > + > > > bool zswap_is_enabled(void) > > > { > > > return zswap_enabled; > > > @@ -241,6 +250,11 @@ static inline struct xarray > > *swap_zswap_tree(swp_entry_t swp) > > > pr_debug("%s pool %s/%s\n", msg, (p)->tfm_name, \ > > > zpool_get_type((p)->zpool)) > > > > > > +__always_inline bool zswap_store_batching_enabled(void) > > > +{ > > > + return __zswap_store_batching_enabled; > > > +} > > > + > > > /********************************* > > > * pool functions > > > **********************************/ > > > -- > > > 2.27.0 > > >