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 58BC8E6C608 for ; Tue, 3 Dec 2024 05:34:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9FE486B007B; Tue, 3 Dec 2024 00:34:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9AE4C6B0083; Tue, 3 Dec 2024 00:34:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 84EF06B0085; Tue, 3 Dec 2024 00:34:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 5ECFD6B007B for ; Tue, 3 Dec 2024 00:34:53 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C0BB5ADE0E for ; Tue, 3 Dec 2024 05:34:52 +0000 (UTC) X-FDA: 82852533036.27.B1A9C74 Received: from mail-yw1-f178.google.com (mail-yw1-f178.google.com [209.85.128.178]) by imf11.hostedemail.com (Postfix) with ESMTP id 8F65840007 for ; Tue, 3 Dec 2024 05:34:38 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=YwcVwjT7; spf=pass (imf11.hostedemail.com: domain of yosryahmed@google.com designates 209.85.128.178 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733204082; 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=pJNCj+uh9klufOQ1xByZwnI7VLbBQeWlMH2kBkaAN4M=; b=DDiF2eYy61dKirOv3k91D6TdGa4d9cUKDanWXxNDPR0MpRUxhVmlHqEhzzDeAfRMmSfolt 6hrjoz6qO68Br7MNO2pUDQGng+JrM7Ig4SLkZtczkcksYi/FVPBLdyNg6nJtjS10aCOnfW aye3GTV4IxFyGmYdrf/27wXJ767EQRI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733204082; a=rsa-sha256; cv=none; b=na8klpNnwAzO7lFqPlM3x8PObmYiQrePNXoe/vVSAry6bA35qDb9Ee/LN4hPALnuWmXCiS 99UnlhunMVVZHjDhbL72FDu0I9OLRVD6ahrn8kZr7nsUrfSmfsLKvdrXa6PCR+9wR/VBIO CRFCfzxeo0xzYg53rXEJpjCjixN4L1Q= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=YwcVwjT7; spf=pass (imf11.hostedemail.com: domain of yosryahmed@google.com designates 209.85.128.178 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yw1-f178.google.com with SMTP id 00721157ae682-6ef4084df4cso45461237b3.1 for ; Mon, 02 Dec 2024 21:34:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733204090; x=1733808890; 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=pJNCj+uh9klufOQ1xByZwnI7VLbBQeWlMH2kBkaAN4M=; b=YwcVwjT71TsFc5eug/JAI8T8LaYXXJloxcL22VWcMQYp/p5DWXehf6jgB38/jX3yb9 n3DsnCkS7EfFESWKkfhckrqu/tt8votHztk1m75yOGEVk+57rEWRM+gfoqcT7cT52pc+ V4L9zSbLzq75mH7siSsMn0jAd2B4U0J/lJEsV7ho7flX1qzEyTO4guQusURs0ZeZRgM8 n0MupGOaZHzOjjp99u71S4mX5Xjgk3kwXAmZyQ41jdDDuYdq8MV/0QRIw7yGMyIdJwTg Cx4eUvh8GyuIj/uo7PK65ClnnxW7n0rL8+2q7K1ZUUbAQH46SNXO6iUPC1hUWL7j5V1y /GuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733204090; x=1733808890; 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=pJNCj+uh9klufOQ1xByZwnI7VLbBQeWlMH2kBkaAN4M=; b=KumXjsqruQAdk8xjzgif2lB7/ek1feAYyhZ+8V1Ab2G6B8qk6xPS9h++N8NZbQEWVD zgpTxIABD+vPIfsbxSP+WpqhSRSaxYYO/rYl1E7aZyAxFzn424Uv8R40qvnEdcZCRSUr KiOGMFSI/arVroFrq5o7rpIHXzAVmu4HPHHET0JMmJO7cDwa2735YlD7VCavx/0UFdog SPPQoDpTld2FGkkKa/NNM10XJFwdfsgIxHGIwm3/RNCJ8ZuFkAv2DXwlgSrWafhpnMes qOMI65kw5ss2Tmc9ufCZLDvxBOoLa0E2wtkG/390wVqhsd8dQfXWExkPVfdefmB/5H76 E2+A== X-Forwarded-Encrypted: i=1; AJvYcCV3VUgVyd9F7B77x1n9GIasjv+R6AOyA0lwhys8ghpU3Zv1Kz4Oa24EAA4351ZX3KILHN+vv8TSPA==@kvack.org X-Gm-Message-State: AOJu0YzREzrbay4z+DTgnFTY/HyexPfZmX1whPKwAO6/aYqapXzoArGf gw5+YDm9RogCuhERt89PUEIvuuoDCQ9JyPlM1A1Bnd9sn/RKWoSVEGYsO2B8eL7HV++JRuxI/9w jE3LAtRBH++wUkJ2PpjNXtTl5/Tj9yIGDEgO6 X-Gm-Gg: ASbGncvLw6iK/Tcu2OtACbtV7LZgIihm3IjvkBNiUv+q3T1aoqaCcqniX42F+Sz2OMc QS6H1r8xsgBXkDlyrhv8iI2yue5lN X-Google-Smtp-Source: AGHT+IFVS0+686xfkJkz3Ni0rcQ6kFCbqnTgmutwtLIbKBx85HDDFrnjFwn9cLymCgTplIULZcE8wd1+dj1nwK7xikM= X-Received: by 2002:a05:690c:9985:b0:6ee:7797:672 with SMTP id 00721157ae682-6eface05a9emr20561557b3.7.1733204089842; Mon, 02 Dec 2024 21:34:49 -0800 (PST) MIME-Version: 1.0 References: <20241127225324.6770-1-kanchana.p.sridhar@intel.com> <20241127225324.6770-2-kanchana.p.sridhar@intel.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 2 Dec 2024 21:34:13 -0800 Message-ID: Subject: Re: [PATCH v1 1/2] mm: zswap: Modified zswap_store_page() to process multiple pages in a folio. 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" , "21cnbao@gmail.com" <21cnbao@gmail.com>, "akpm@linux-foundation.org" , "Feghali, Wajdi K" , "Gopal, Vinodh" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 8F65840007 X-Stat-Signature: 1bwnhn3qcrts3icc1q3k9ybutobcbi9u X-Rspam-User: X-HE-Tag: 1733204078-459533 X-HE-Meta: U2FsdGVkX1+9vAGBdp4f375GthLRM1BwzkmqCaRWjjQ+XGaK8ckXS82CjS9TZ7OUculQBLiNrcyAdFpjpvZY9qAr4qRyLOGCmcGp57ud0P7iloQLHk1p16IdO5c0EA5ClNPRsQKbExiPTJ3pgfi4p95xmx0h0l+bWFJfoI1aoPGn8NFa5+byAFOzPcLSBPOV44WqrwiSDt5L/qpq+4j8fKH+1iBFZxvy1DwbrZ82fWdxCiYPjhROOgmWNXVNA4huLJJs7zskVG9TUawP4LBm2x4vJJSG89ie/1k0iRH0xBIRubpngfGngI7e0epyr6GI9gMnYZiINeXmQe9C22gG4QJdCsDJt0G2+T+hg8dc4BW2fhsJ0cNo//Q4dARLQ0MZBoCDF3KXJbqJoROzbOg7v0NSKoowCNWR7oQ4/0RnOO4ChvTNia08+0/NnvsULMzDxOdUdsYhg+wLh+A35SY3OeSjM+bS7JmRpKnbr/rtYFi8WrzlrUGJ+Wam9TIYBCy0bSkPq9RbrM7sO32et4baSv+KvZr/sakH1Bbuuro4cwPuE1u2KV7f85vknlHAaRYpIdv8SFJBi2x55ETXl9YQV0ViR4Bi26nKJAhBCKxA3TjmgVUMi9uDmNZZRDLM2zUm/UB8cK9ftgEfRP6YY80WXNhrfpRpeZBwdxy7ES5OVXHhq8pp/8i0rYz2V3lh+CPGCtYGZfboT1R7HlsenMH2CZGO1z3dO6e/DT2290YI2iQSBtOPbRwiwwvfNjRdK5tEfplO5oko2l4XgQMnr3dnjDlU0BXiRa3Eqamkp5bAi2sGvmlB4gl/1lThq/YLCkRXktXhfM4N7NP/IoVL6tm7AtuDMTT+hHuCs+e17u9zaglZCPlKkIbMWquGglBOjZm5r2pGLCHhYiCJdxve7UdrWFDyf9AjFFKOABXLYg/Iya6Zmk8t8+yltZyh5je5/bW+vSNo11rzvRxgsMfiVrQ sjbyQzLQ YV8TJEZ3JLXbwqRWAGu1/X/UoIH3rM6oTyxSXBpi27dVumFmCvpOgcx2J8MWgoqcMoinLpoEhZcuL8BiMPjJhuhhivl9MERpHLIcCWQIyICuScwJn+RjVqBzsVPDN/5tXxPdJnlqDpi/qnuzd2m+4MIFmzwsGEPCl4Eh25I2R31ZjE+b0eIJl7LSKGKEBI63B0DFckpAb+ws31fFzt8cMOl6noGle62XQFjkKnShfwno5VVKNs1bOfnx729fM3BJzZLrB4C+t6OVTS7s2WdI+Umok+vwSzVgQYfKNC5IC9qWGEfOs+1WFIDAGCJ8lhljIe8UR1wqzz9LUZbTIJx48F4KjaFWF8BWhy3ZypzoUIBsrRzC9y4SzCRZA/Tjo6aPevIpkG4npwnSjvCoMLRTYcFp8e/8G8hGvPWGrt49l1egIV6waT6GbG/YUTb+c3wt7Oj8FWr4b2kmNEJBU1xF1Pwbo3GHjkOltfi2Z5Vy33Si6OYOub8WVbAJekJQLDxVyNu1NBQr12qmzZHdg96zUANFuERbIf5WhfYWD50NslqPO/21kIfr8dCQUB0qed3f77ARg8CTObEqwhz4MUoeyBQ2mamW8wM0+p7F/q0j0QsgiVJ8= 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 Mon, Dec 2, 2024 at 5:13=E2=80=AFPM Sridhar, Kanchana P wrote: > > > > -----Original Message----- > > From: Yosry Ahmed > > Sent: Monday, December 2, 2024 11:34 AM > > 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; 21cnbao@gmail.com; > > akpm@linux-foundation.org; Feghali, Wajdi K = ; > > Gopal, Vinodh > > Subject: Re: [PATCH v1 1/2] mm: zswap: Modified zswap_store_page() to > > process multiple pages in a folio. > > > > On Wed, Nov 27, 2024 at 2:53=E2=80=AFPM Kanchana P Sridhar > > wrote: > > > > > > Modified zswap_store() to store the folio in batches of > > > SWAP_CRYPTO_BATCH_SIZE pages. Accordingly, refactored > > zswap_store_page() > > > into zswap_store_pages() that processes a range of pages in the folio= . > > > zswap_store_pages() is a vectorized version of zswap_store_page(). > > > > > > For now, zswap_store_pages() will sequentially compress these pages w= ith > > > zswap_compress(). > > > > > > These changes are follow-up to code review comments received for [1],= and > > > are intended to set up zswap_store() for batching with Intel IAA. > > > > > > [1]: https://patchwork.kernel.org/project/linux- > > mm/patch/20241123070127.332773-11-kanchana.p.sridhar@intel.com/ > > > > > > Signed-off-by: Kanchana P Sridhar > > > --- > > > include/linux/zswap.h | 1 + > > > mm/zswap.c | 154 ++++++++++++++++++++++++----------------= -- > > > 2 files changed, 88 insertions(+), 67 deletions(-) > > > > > > diff --git a/include/linux/zswap.h b/include/linux/zswap.h > > > index d961ead91bf1..05a81e750744 100644 > > > --- a/include/linux/zswap.h > > > +++ b/include/linux/zswap.h > > > @@ -7,6 +7,7 @@ > > > > > > struct lruvec; > > > > > > +#define SWAP_CRYPTO_BATCH_SIZE 8UL > > > extern atomic_long_t zswap_stored_pages; > > > > > > #ifdef CONFIG_ZSWAP > > > diff --git a/mm/zswap.c b/mm/zswap.c > > > index f6316b66fb23..b09d1023e775 100644 > > > --- a/mm/zswap.c > > > +++ b/mm/zswap.c > > > @@ -1409,78 +1409,96 @@ static void shrink_worker(struct work_struct > > *w) > > > * main API > > > **********************************/ > > > > > > -static ssize_t zswap_store_page(struct page *page, > > > - struct obj_cgroup *objcg, > > > - struct zswap_pool *pool) > > > +/* > > > + * Store multiple pages in @folio, starting from the page at index @= si up to > > > + * and including the page at index @ei. > > > + */ > > > +static ssize_t zswap_store_pages(struct folio *folio, > > > + long si, > > > + long ei, > > > + struct obj_cgroup *objcg, > > > + struct zswap_pool *pool) > > > { > > > - swp_entry_t page_swpentry =3D page_swap_entry(page); > > > + struct page *page; > > > + swp_entry_t page_swpentry; > > > struct zswap_entry *entry, *old; > > > + size_t compressed_bytes =3D 0; > > > + u8 nr_pages =3D ei - si + 1; > > > + u8 i; > > > + > > > + for (i =3D 0; i < nr_pages; ++i) { > > > + page =3D folio_page(folio, si + i); > > > + page_swpentry =3D page_swap_entry(page); > > > + > > > + /* allocate entry */ > > > + entry =3D zswap_entry_cache_alloc(GFP_KERNEL, > > page_to_nid(page)); > > > + if (!entry) { > > > + zswap_reject_kmemcache_fail++; > > > + return -EINVAL; > > > + } > > > > I think this patch is wrong on its own, for example if an allocation > > fails in the above loop we exit without cleaning up previous > > allocations. I think it's fixed in patch 2 but we cannot introduce > > I think there might be a misunderstanding. zswap_store_pages() will > clean up local resources allocated during an iteration of the for loop, > upon an error in that iteration. If you see the "goto store_failed" and > "goto compress_failed" this would explain what I mean. If an allocation > fails for an iteration, we simply return -EINVAL, and zswap_store() > will goto the "put_pool" label with "ret" still false, which will delete > all zswap entries for the folio (allocated up until the error iteration i= n > zswap_store_pages(); or potentially already in the xarray). > > Hence, there is no bug and each of the two patches are correct by > themselves AFAICT, but please let me know if I am missing anything. Thank= s! Uh yes, the cleanup is in zswap_store(). > > > bugs in-between patches. I think the helpers in patch 2 don't really > > help as I mentioned. Please combine the changes and keep them in the > > main series (unless you have a reason not to). > > Sure. As noted in my earlier response to comments received for patch 2, > I can either inline all iterations or create helpers for all iterations o= ver > the pages in a batch. Appreciate your suggestions on which would be a > better approach. I think leaving them open-coded will be clearer for now. We can revisit the code organization later if it gets out of hand.