From: Yosry Ahmed <yosryahmed@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Nhat Pham <nphamcs@gmail.com>,
Chengming Zhou <chengming.zhou@linux.dev>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Yosry Ahmed <yosryahmed@google.com>
Subject: [PATCH 2/2] mm: zswap: remove nr_zswap_stored atomic
Date: Wed, 20 Mar 2024 02:08:23 +0000 [thread overview]
Message-ID: <20240320020823.337644-2-yosryahmed@google.com> (raw)
In-Reply-To: <20240320020823.337644-1-yosryahmed@google.com>
zswap_nr_stored is used to maintain the number of stored pages in zswap
that are not same-filled pages. It is used in zswap_shrinker_count() to
scale the number of freeable compressed pages by the compression ratio.
That is, to reduce the amount of writeback from zswap with higher
compression ratios as the ROI from IO diminishes.
However, the need for this counter is questionable due to two reasons:
- It is redundant. The value can be inferred from (zswap_stored_pages -
zswap_same_filled_pages).
- When memcgs are enabled, we use memcg_page_state(memcg,
MEMCG_ZSWAPPED), which includes same-filled pages anyway (i.e.
equivalent to zswap_stored_pages).
Use zswap_stored_pages instead in zswap_shrinker_count() to keep things
consistent whether memcgs are enabled or not, and add a comment about
the number of freeable pages possibly being scaled down more than it
should if we have lots of same-filled pages (i.e. inflated compression
ratio).
Remove nr_zswap_stored and one atomic operation in the store and free
paths.
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
mm/zswap.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 323f1dea43d22..ffcfce05a4408 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -181,8 +181,6 @@ struct zswap_pool {
/* Global LRU lists shared by all zswap pools. */
static struct list_lru zswap_list_lru;
-/* counter of pages stored in all zswap pools. */
-static atomic_t zswap_nr_stored = ATOMIC_INIT(0);
/* The lock protects zswap_next_shrink updates. */
static DEFINE_SPINLOCK(zswap_shrink_lock);
@@ -880,7 +878,6 @@ static void zswap_entry_free(struct zswap_entry *entry)
else {
zswap_lru_del(&zswap_list_lru, entry);
zpool_free(zswap_find_zpool(entry), entry->handle);
- atomic_dec(&zswap_nr_stored);
zswap_pool_put(entry->pool);
}
if (entry->objcg) {
@@ -1305,7 +1302,7 @@ static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
#else
/* use pool stats instead of memcg stats */
nr_backing = zswap_total_pages();
- nr_stored = atomic_read(&zswap_nr_stored);
+ nr_stored = atomic_read(&zswap_stored_pages);
#endif
if (!nr_stored)
@@ -1325,6 +1322,11 @@ static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
* This ensures that the better zswap compresses memory, the fewer
* pages we will evict to swap (as it will otherwise incur IO for
* relatively small memory saving).
+ *
+ * The memory saving factor calculated here takes same-filled pages into
+ * account, but those are not freeable since they almost occupy no
+ * space. Hence, we may scale nr_freeable down a little bit more than we
+ * should if we have a lot of same-filled pages.
*/
return mult_frac(nr_freeable, nr_backing, nr_stored);
}
@@ -1570,7 +1572,6 @@ bool zswap_store(struct folio *folio)
if (entry->length) {
INIT_LIST_HEAD(&entry->lru);
zswap_lru_add(&zswap_list_lru, entry);
- atomic_inc(&zswap_nr_stored);
}
spin_unlock(&tree->lock);
--
2.44.0.291.gc1ea87d7ee-goog
next prev parent reply other threads:[~2024-03-20 2:08 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-20 2:08 [PATCH 1/2] mm: zswap: increase shrinking protection for zswap swapins only Yosry Ahmed
2024-03-20 2:08 ` Yosry Ahmed [this message]
2024-03-21 21:09 ` [PATCH 2/2] mm: zswap: remove nr_zswap_stored atomic Yosry Ahmed
2024-03-21 23:50 ` Nhat Pham
2024-03-21 23:57 ` Yosry Ahmed
2024-03-20 9:50 ` [PATCH 1/2] mm: zswap: increase shrinking protection for zswap swapins only Johannes Weiner
2024-03-20 14:48 ` Nhat Pham
2024-03-20 19:32 ` Yosry Ahmed
2024-03-20 19:28 ` Yosry Ahmed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240320020823.337644-2-yosryahmed@google.com \
--to=yosryahmed@google.com \
--cc=akpm@linux-foundation.org \
--cc=chengming.zhou@linux.dev \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nphamcs@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox