The patch titled
Subject: mm: zswap: fix double invalidate with exclusive loads
has been added to the -mm mm-unstable branch. Its filename is
mm-zswap-fix-double-invalidate-with-exclusive-loads.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-zswap-fix-double-invalidate-with-exclusive-loads.patch
This patch will later appear in the mm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
Subject: mm: zswap: fix double invalidate with exclusive loads
Date: Wed, 21 Jun 2023 09:30:09 +0000
If exclusive loads are enabled for zswap, we invalidate the entry before
returning from zswap_frontswap_load(), after dropping the local reference.
However, the tree lock is dropped during decompression after the local
reference is acquired, so the entry could be invalidated before we drop
the local ref. If this happens, the entry is freed once we drop the local
ref, and zswap_invalidate_entry() tries to invalidate an already freed
entry.
Fix this by:
(a) Making sure zswap_invalidate_entry() is always called with a local
ref held, to avoid being called on a freed entry.
(b) Making sure zswap_invalidate_entry() only drops the ref if the entry
was actually on the rbtree. Otherwise, another invalidation could
have already happened, and the initial ref is already dropped.
With these changes, there is no need to check that there is no need to
make sure the entry still exists in the tree in zswap_reclaim_entry()
before invalidating it, as zswap_reclaim_entry() will make this check
internally.
Link: https://lkml.kernel.org/r/20230621093009.637544-1-yosryahmed@xxxxxxxxxx
Fixes: b9c91c43412f ("mm: zswap: support exclusive loads")
Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
Reported-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx>
Cc: Dan Streetman <ddstreet@xxxxxxxx>
Cc: Domenico Cerasuolo <cerasuolodomenico@xxxxxxxxx>
Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Nhat Pham <nphamcs@xxxxxxxxx>
Cc: Seth Jennings <sjenning@xxxxxxxxxx>
Cc: Vitaly Wool <vitaly.wool@xxxxxxxxxxxx>
Cc: Yu Zhao <yuzhao@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
mm/zswap.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
--- a/mm/zswap.c~mm-zswap-fix-double-invalidate-with-exclusive-loads
+++ a/mm/zswap.c
@@ -355,12 +355,14 @@ static int zswap_rb_insert(struct rb_roo
return 0;
}
-static void zswap_rb_erase(struct rb_root *root, struct zswap_entry *entry)
+static bool zswap_rb_erase(struct rb_root *root, struct zswap_entry *entry)
{
if (!RB_EMPTY_NODE(&entry->rbnode)) {
rb_erase(&entry->rbnode, root);
RB_CLEAR_NODE(&entry->rbnode);
+ return true;
}
+ return false;
}
/*
@@ -599,14 +601,16 @@ static struct zswap_pool *zswap_pool_fin
return NULL;
}
+/*
+ * If the entry is still valid in the tree, drop the initial ref and remove it
+ * from the tree. This function must be called with an additional ref held,
+ * otherwise it may race with another invalidation freeing the entry.
+ */
static void zswap_invalidate_entry(struct zswap_tree *tree,
struct zswap_entry *entry)
{
- /* remove from rbtree */
- zswap_rb_erase(&tree->rbroot, entry);
-
- /* drop the initial reference from entry creation */
- zswap_entry_put(tree, entry);
+ if (zswap_rb_erase(&tree->rbroot, entry))
+ zswap_entry_put(tree, entry);
}
static int zswap_reclaim_entry(struct zswap_pool *pool)
@@ -659,8 +663,7 @@ static int zswap_reclaim_entry(struct zs
* swapcache. Drop the entry from zswap - unless invalidate already
* took it out while we had the tree->lock released for IO.
*/
- if (entry == zswap_rb_search(&tree->rbroot, swpoffset))
- zswap_invalidate_entry(tree, entry);
+ zswap_invalidate_entry(tree, entry);
put_unlock:
/* Drop local reference */
@@ -1466,7 +1469,6 @@ stats:
count_objcg_event(entry->objcg, ZSWPIN);
freeentry:
spin_lock(&tree->lock);
- zswap_entry_put(tree, entry);
if (!ret && zswap_exclusive_loads_enabled) {
zswap_invalidate_entry(tree, entry);
*exclusive = true;
@@ -1475,6 +1477,7 @@ freeentry:
list_move(&entry->lru, &entry->pool->lru);
spin_unlock(&entry->pool->lru_lock);
}
+ zswap_entry_put(tree, entry);
spin_unlock(&tree->lock);
return ret;
_
Patches currently in -mm which might be from yosryahmed@xxxxxxxxxx are
mm-zswap-fix-double-invalidate-with-exclusive-loads.patch
