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 63C92E6C5F5 for ; Tue, 3 Dec 2024 03:23:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B6F2E6B007B; Mon, 2 Dec 2024 22:23:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B1F146B0083; Mon, 2 Dec 2024 22:23:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CDA86B0085; Mon, 2 Dec 2024 22:23:55 -0500 (EST) 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 810866B007B for ; Mon, 2 Dec 2024 22:23:55 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 23ACAA04F7 for ; Tue, 3 Dec 2024 03:23:55 +0000 (UTC) X-FDA: 82852203462.02.722E6B9 Received: from out-171.mta0.migadu.com (out-171.mta0.migadu.com [91.218.175.171]) by imf26.hostedemail.com (Postfix) with ESMTP id 9BE8E140002 for ; Tue, 3 Dec 2024 03:23:43 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=LbSI0OVY; spf=pass (imf26.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.171 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733196220; 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=Wj+5GXCu0vCAWHxv1tYtK3ssQIvbHpWALbj9FVJY9Ro=; b=Nl+F7Cyf4sYYntQXA3rhBJvJxkQDeyRkNH1W6nGehzmY523zaeI3jB5JrjUHBLRpqozg9r qKh+f7llEiwqO0spSGo7LvYDgz9wxu4o1hFNCzw4x6qeHceLRKvWxLRjuW26dQv10mR70M vM6BB9aM9gkkRAp61j81/XcexNXKS0c= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733196220; a=rsa-sha256; cv=none; b=eifaklnBccJyMfSgTQOr9/9fNdRjXnvtDO+E8Z1mwnUkxz38hTluwUPJC1szVWs31ZeX8V VQUv30ek4roRpEED7lzE7b8tcUn7qjNG0gmxLSJ25fhWDKyEaUVtbMXNPsIDff5PolVwX3 LQ7yi7UUQyM4iNz9CKvMoqj3W455KOk= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=LbSI0OVY; spf=pass (imf26.hostedemail.com: domain of chengming.zhou@linux.dev designates 91.218.175.171 as permitted sender) smtp.mailfrom=chengming.zhou@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1733196230; h=from:from: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; bh=Wj+5GXCu0vCAWHxv1tYtK3ssQIvbHpWALbj9FVJY9Ro=; b=LbSI0OVYtJnBrx+mEg4u1wsqtcmCXYMfLfARm/pPW0w4ZRQh4S/I/GaOMFUXrTegzxX/Oc Mo4+CgwnaYB+KtF5r4UqnQVRyGDKZeByhFLpIvvYLAJg6hqDd8QHWUVbI92J04wVYuDQlA FPhi86/thgzzlf01WoC/zq9uNTQvEYU= Date: Tue, 3 Dec 2024 11:23:37 +0800 MIME-Version: 1.0 Subject: Re: [PATCH v1 1/2] mm: zswap: Modified zswap_store_page() to process multiple pages in a folio. To: Kanchana P Sridhar , linux-kernel@vger.kernel.org, linux-mm@kvack.org, hannes@cmpxchg.org, yosryahmed@google.com, nphamcs@gmail.com, usamaarif642@gmail.com, ryan.roberts@arm.com, 21cnbao@gmail.com, akpm@linux-foundation.org Cc: wajdi.k.feghali@intel.com, vinodh.gopal@intel.com References: <20241127225324.6770-1-kanchana.p.sridhar@intel.com> <20241127225324.6770-2-kanchana.p.sridhar@intel.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Chengming Zhou In-Reply-To: <20241127225324.6770-2-kanchana.p.sridhar@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 9BE8E140002 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: n1bxekj7u8iqsrtedqpqa6q1hhpokkct X-HE-Tag: 1733196223-661582 X-HE-Meta: U2FsdGVkX1/fnK4Xrvu8ijx7ROomKWDo6uUzU5jDAZIc0zLAoUvrrfTU6DdMhZFj1UYSTDpu4H4bZQNXk267zF5Jpz9kF9gdyyuc0skL2KVAwjED2Ud8KVBc+uZz4BoloSebU48r6JxIXRR6BV9P5kYUfTWCoylpOUMbHN8hnUllbbgyyJ2rmz9Dzu5QibDQr1Tlg4OGTaLzc3czEIvwNtQ8qaZ408xYhdF5gIVIwbf+nR8myw306HFtE9rspZN6L7M7HWcuk9djSbp62uMWCkvWV2708qcs3i7YypE8bdy4cRSW0vvCU13c1pd5q0KxCNQ9UlJDP3kdE8xTXwLatJBoexreZCdDzxJThvy9bJOLFomsYYLa1xPeImn527mwMOlYdP/loNr5vDrSwcMy+RbxcYWeG7RJH+oTHUw5FQnGJxsjmPtUzInDc5tZPSIbCrNLx73uV+cVJCrrXvaiZruV/4OZnNSAEWuddhqoGEkBrtOzebsxpwAF77+TOjmiNlLoGW/4R0OwlZ+4wKTDxJPw6jcp9fDYUO182zQ6WpuX9FV7K/pqK8zvQgD5zYLEqBUttp/h20DW3qRnfM6EK7xK/aeaQ80aa0slrv704TiTM7qIsTcitfvkNH43tWWp63liGEUoFm7R7xhHZuc4P9gVD8JoxiOO4nqRqyiR5TPrYQ0sBKsyTXBVqXENPRNdlP+l0KMWkawiOu+IKuvPVfChddOIq2abfNnT/dnaPkeIuAiG5LD+jXbMhBqKq7C/D1Fd0Q1aooPpfPGlp2A+My7Cps62sse+KqrnWw/f3WUIv9fEydpXKMWobFJjjYTGujJPgtdfHEsFLCv8KVRz5/PuBCXPKg4QkUiLQYXtxLqtGwT2SPbZHMGf6yUqo+WjO/A74DnKFWTsSUxx0kcF3sDeUBQGCVlT0HYPG4TM6DYnM27ZOGYthhWuGpSv0EWrwCNd6uprnVGVuRnWu/w sbFykXJ/ wmBIMTMHkYis6pPZwOXgdmKhoN9s7Vu4TuLvtoUGNQiNjT5C3s7r5eT3fGjZ6Mv+QYeO8wcJpIMalEifRutmsqwnLEi7jc9DIY5CdK03mAKnJpubY5699rX7z0HoQ1WiZ9vl0DIluq8LNkM4uqqekz5oIDNUzzxwLpk9X9oGl1F+IdCMJ4a6I1jHv9eLpqNq1pfIsoqeg4U/7C7guA+F+0C+zucUpxT+5z9JCpJmYihpiw9bkRYjEBUlkiA== 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/11/28 06:53, 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(). Maybe I missed something, but this refactor change looks confused to me. Why not zswap_store_pages() just reuse the existing zswap_store_page()? ``` zswap_store_pages() compressed_bytes = 0 for each page in this batch size = zswap_store_page(page) if (size < 0) return size; compressed_bytes += size; return compressed_bytes; ``` Thanks. > > For now, zswap_store_pages() will sequentially compress these pages with > 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 = page_swap_entry(page); > + struct page *page; > + swp_entry_t page_swpentry; > struct zswap_entry *entry, *old; > + size_t compressed_bytes = 0; > + u8 nr_pages = ei - si + 1; > + u8 i; > + > + for (i = 0; i < nr_pages; ++i) { > + page = folio_page(folio, si + i); > + page_swpentry = page_swap_entry(page); > + > + /* allocate entry */ > + entry = zswap_entry_cache_alloc(GFP_KERNEL, page_to_nid(page)); > + if (!entry) { > + zswap_reject_kmemcache_fail++; > + return -EINVAL; > + } > > - /* allocate entry */ > - entry = zswap_entry_cache_alloc(GFP_KERNEL, page_to_nid(page)); > - if (!entry) { > - zswap_reject_kmemcache_fail++; > - return -EINVAL; > - } > - > - if (!zswap_compress(page, entry, pool)) > - goto compress_failed; > + if (!zswap_compress(page, entry, pool)) > + goto compress_failed; > > - old = xa_store(swap_zswap_tree(page_swpentry), > - swp_offset(page_swpentry), > - entry, GFP_KERNEL); > - if (xa_is_err(old)) { > - int err = xa_err(old); > + old = xa_store(swap_zswap_tree(page_swpentry), > + swp_offset(page_swpentry), > + entry, GFP_KERNEL); > + if (xa_is_err(old)) { > + int err = xa_err(old); > > - WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); > - zswap_reject_alloc_fail++; > - goto store_failed; > - } > + WARN_ONCE(err != -ENOMEM, "unexpected xarray error: %d\n", err); > + zswap_reject_alloc_fail++; > + goto store_failed; > + } > > - /* > - * We may have had an existing entry that became stale when > - * the folio was redirtied and now the new version is being > - * swapped out. Get rid of the old. > - */ > - if (old) > - zswap_entry_free(old); > + /* > + * We may have had an existing entry that became stale when > + * the folio was redirtied and now the new version is being > + * swapped out. Get rid of the old. > + */ > + if (old) > + zswap_entry_free(old); > > - /* > - * The entry is successfully compressed and stored in the tree, there is > - * no further possibility of failure. Grab refs to the pool and objcg. > - * These refs will be dropped by zswap_entry_free() when the entry is > - * removed from the tree. > - */ > - zswap_pool_get(pool); > - if (objcg) > - obj_cgroup_get(objcg); > + /* > + * The entry is successfully compressed and stored in the tree, there is > + * no further possibility of failure. Grab refs to the pool and objcg. > + * These refs will be dropped by zswap_entry_free() when the entry is > + * removed from the tree. > + */ > + zswap_pool_get(pool); > + if (objcg) > + obj_cgroup_get(objcg); > > - /* > - * We finish initializing the entry while it's already in xarray. > - * This is safe because: > - * > - * 1. Concurrent stores and invalidations are excluded by folio lock. > - * > - * 2. Writeback is excluded by the entry not being on the LRU yet. > - * The publishing order matters to prevent writeback from seeing > - * an incoherent entry. > - */ > - entry->pool = pool; > - entry->swpentry = page_swpentry; > - entry->objcg = objcg; > - entry->referenced = true; > - if (entry->length) { > - INIT_LIST_HEAD(&entry->lru); > - zswap_lru_add(&zswap_list_lru, entry); > - } > + /* > + * We finish initializing the entry while it's already in xarray. > + * This is safe because: > + * > + * 1. Concurrent stores and invalidations are excluded by folio lock. > + * > + * 2. Writeback is excluded by the entry not being on the LRU yet. > + * The publishing order matters to prevent writeback from seeing > + * an incoherent entry. > + */ > + entry->pool = pool; > + entry->swpentry = page_swpentry; > + entry->objcg = objcg; > + entry->referenced = true; > + if (entry->length) { > + INIT_LIST_HEAD(&entry->lru); > + zswap_lru_add(&zswap_list_lru, entry); > + } > > - return entry->length; > + compressed_bytes += entry->length; > + continue; > > store_failed: > - zpool_free(pool->zpool, entry->handle); > + zpool_free(pool->zpool, entry->handle); > compress_failed: > - zswap_entry_cache_free(entry); > - return -EINVAL; > + zswap_entry_cache_free(entry); > + return -EINVAL; > + } > + > + return compressed_bytes; > } > > bool zswap_store(struct folio *folio) > @@ -1492,7 +1510,7 @@ bool zswap_store(struct folio *folio) > struct zswap_pool *pool; > size_t compressed_bytes = 0; > bool ret = false; > - long index; > + long si, ei, incr = SWAP_CRYPTO_BATCH_SIZE; > > VM_WARN_ON_ONCE(!folio_test_locked(folio)); > VM_WARN_ON_ONCE(!folio_test_swapcache(folio)); > @@ -1526,11 +1544,13 @@ bool zswap_store(struct folio *folio) > mem_cgroup_put(memcg); > } > > - for (index = 0; index < nr_pages; ++index) { > - struct page *page = folio_page(folio, index); > + /* Store the folio in batches of SWAP_CRYPTO_BATCH_SIZE pages. */ > + for (si = 0, ei = min(si + incr - 1, nr_pages - 1); > + ((si < nr_pages) && (ei < nr_pages)); > + si = ei + 1, ei = min(si + incr - 1, nr_pages - 1)) { > ssize_t bytes; > > - bytes = zswap_store_page(page, objcg, pool); > + bytes = zswap_store_pages(folio, si, ei, objcg, pool); > if (bytes < 0) > goto put_pool; > compressed_bytes += bytes; > @@ -1565,9 +1585,9 @@ bool zswap_store(struct folio *folio) > struct zswap_entry *entry; > struct xarray *tree; > > - for (index = 0; index < nr_pages; ++index) { > - tree = swap_zswap_tree(swp_entry(type, offset + index)); > - entry = xa_erase(tree, offset + index); > + for (si = 0; si < nr_pages; ++si) { > + tree = swap_zswap_tree(swp_entry(type, offset + si)); > + entry = xa_erase(tree, offset + si); > if (entry) > zswap_entry_free(entry); > }