Skip to contentSkip to conversation

mm: store zswap entries in swap table (Pointer entry)

RFC PATCH In review Baoquan He 4 participants 13 messages Jul 7 – Jul 13

Subsystems (3)
lore
Share & tools
lore

Baoquan He

author reviewer
#0 lore raw

cover mm: store zswap entries in swap table (Pointer entry)

Currently zswap uses a per-swap-device xarray to map swap offsets to zswap_entry pointers. This introduces an additional tree walk on every zswap entry access.

This series replaces the xarray with a "Pointer" entry type stored directly in the swap cluster table. The new swp_tb_t variant embeds a zswap_entry pointer plus a entry count, eliminating the xarray and its lookup overhead entirely.

Design overview
---------------
A swap table slot can now be one of five types:

  NULL:    |---------------- 0 ---------------|
  Shadow:  |SWAP_COUNT|Z|---- SHADOW_VAL ---|1|
  PFN:     |SWAP_COUNT|Z|------ PFN -------|10|
  Pointer: |----------- Pointer ----------|100|
  Bad:     |------------- 1 -------------|1000|

When zswap compresses a page, it saves the original swap table entry (PFN or Shadow) in zswap_entry->swp_tb_val, then writes the Pointer entry into the swap table. The swap count and flags remain accessible via helper functions that read them from swp_tb_val.

The zswap_entry is threaded through the swapin call chain via a new void **zentry parameter, so zswap_load() receives it directly instead of doing an xarray lookup.

Performance
-----------
[~]# free -h
total used free shared buff/cache available

Mem:           3.8Gi       155Mi       3.4Gi       656Ki       315Mi       3.7Gi
Swap:          4.0Gi        54Mi       3.9Gi

Scalability benchmark: 'usemem -O -y -x -n 1 5G', 10 runs under cgroup MemoryMax=2G MemorySwapMax=3G. zswap enabled with zstd compressor.

Kernel A: cfb8731f5396 (xarray-based lookup, baseline) Kernel B: (Pointer entry)

5G test — the gap widens significantly as swap pressure grows:

     Metric           xarray(A)       Pointer(B)
    --------------------------------------------------
    Avg throughput      709,556 KB/s    792,681 KB/s  (+11.7%)
    Avg free time       167,994 µs     132,896 µs   (-20.9%)

Free time trend across 10 rounds at 5G:

    Pointer: 126→124→126→126→124→129→138→145→142→148  (gradual, +17%)
    xarray:  136→139→136→160→162→161→159→230→219→178  (collapse, +31%)

xarray degrades sharply in the later rounds (last 3 avg free time 209 ms vs Pointer's 145 ms, 1.44× worse) as the radix tree insertion/deletion cost grows non-linearly under sustained swap churn. Pointer entry avoids this entirely by looking up entries directly in the swap cluster table, which is already hot in cache.

Baoquan He (7):
mm/swap_table: add Pointer type support for zswap entry storage mm/zswap: add swp_tb_val to zswap_entry and Pointer accessor helpers mm/swap_state: do zswap conversion in __swap_cache_do_del_folio mm/zswap, mm/swap_state: handle Pointer entries in add_folio path mm/swapfile: handle Pointer entries in count/dup/put paths mm/zswap, mm/swap_state: migrate zswap to use swap_table for entry
indexing
mm/zswap, mm/swap_state: replace xarray indexing with swap table
lookups

 include/linux/zswap.h |   4 +-
 mm/page_io.c          |   4 +-
 mm/swap.h             |  52 +++++-
 mm/swap_state.c       | 117 +++++++++++---
 mm/swap_table.h       |  35 ++++-
 mm/swapfile.c         | 107 ++++++++++---
 mm/zswap.c            | 358 +++++++++++++++++++++++++++++-------------
 7 files changed, 508 insertions(+), 169 deletions(-)
Signature · 1 line
2.54.0
author reviewer
#1 lore raw

RFC PATCH 1/7 mm/swap_table: add Pointer type support for zswap entry storage

Add the Pointer type infrastructure to swap table entries, enabling direct storage of zswap_entry pointers in swap_table entries. The 0b100 marker in the low 3 bits distinguishes pointer entries from NULL (0), Shadow (0b1), and PFN (0b10) entries.

Add helpers:
- SWP_TB_POINTER_MARK: the 0b100 marker constant
- swp_tb_is_pointer(): type check for Pointer entries
- pointer_to_swp_tb(): encode a pointer into a swap table entry
- swp_tb_to_pointer(): decode a swap table entry back to a pointer

---
 mm/swap_table.h | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
mm/swap_table.h +31 -4
index e6613e62f8d0..6318bf8aa76b 100644
@@ -28,7 +28,7 @@ struct swap_memcg_table {
  * NULL:     |---------------- 0 ---------------| - Free slot
  * Shadow:   |SWAP_COUNT|Z|---- SHADOW_VAL ---|1| - Swapped out slot
  * PFN:      |SWAP_COUNT|Z|------ PFN -------|10| - Cached slot
- * Pointer:  |----------- Pointer ----------|100| - (Unused)
+ * Pointer:  |----------- Pointer ----------|100| - Zswap compressed entry
  * Bad:      |------------- 1 -------------|1000| - Bad slot
  *
  * COUNT is `SWP_TB_COUNT_BITS` long, Z is the `SWP_TB_ZERO_FLAG` bit,
@@ -49,9 +49,11 @@ struct swap_memcg_table {
  * - PFN: Swap slot is in use, and cached. Memcg info is recorded on the page
  *   struct.
  *
- * - Pointer: Unused yet. `0b100` is reserved for potential pointer usage
- *   because only the lower three bits can be used as a marker for 8 bytes
- *   aligned pointers.
+ * - Pointer: Stores a direct pointer to a zswap_entry when the swap slot's
+ *   compressed data resides in zswap. The swap count (if any) is stored in
+ *   the zswap_entry struct rather than inline, since the pointer occupies
+ *   all upper bits. `0b100` is used as the marker because zswap_entry
+ *   pointers are at least 8-byte aligned, leaving the lower three bits free.
  *
  * - Bad: Swap slot is reserved, protects swap header or holes on swap devices.
  */
@@ -78,6 +80,9 @@ struct swap_memcg_table {
 /* The first flag is zero bit (SWAP_TABLE_HAS_ZEROFLAG) */
 #define SWP_TB_ZERO_FLAG	BIT(BITS_PER_LONG - SWP_TB_FLAGS_BITS)
 
+/* Pointer: zswap_entry pointer stored directly, ends with 0b100 */
+#define SWP_TB_POINTER_MARK	0b100UL
+
 /* Bad slot: ends with 0b1000 and rests of bits are all 1 */
 #define SWP_TB_BAD		((~0UL) << 3)
 
@@ -166,6 +171,11 @@ static inline bool swp_tb_is_bad(unsigned long swp_tb)
 	return swp_tb == SWP_TB_BAD;
 }
 
+static inline bool swp_tb_is_pointer(unsigned long swp_tb)
+{
+	return (swp_tb & (BIT(3) - 1)) == SWP_TB_POINTER_MARK;
+}
+
 static inline bool swp_tb_is_countable(unsigned long swp_tb)
 {
 	return (swp_tb_is_shadow(swp_tb) || swp_tb_is_folio(swp_tb) ||
@@ -188,6 +198,23 @@ static inline void *swp_tb_to_shadow(unsigned long swp_tb)
 	return (void *)(swp_tb & ~SWP_TB_FLAGS_MASK);
 }
 
+static inline unsigned long pointer_to_swp_tb(void *ptr)
+{
+	unsigned long val = (unsigned long)ptr;
+
+	BUILD_BUG_ON(sizeof(unsigned long) != sizeof(void *));
+	/* Pointers are 8-byte aligned, low 3 bits must be clear */
+	VM_WARN_ON_ONCE(val & (BIT(3) - 1));
+	return val | SWP_TB_POINTER_MARK;
+}
+
+static inline void *swp_tb_to_pointer(unsigned long swp_tb)
+{
+	VM_WARN_ON(!swp_tb_is_pointer(swp_tb));
+	/* Clear the low 3-bit marker to recover the original pointer */
+	return (void *)(swp_tb & ~((unsigned long)(BIT(3) - 1)));
+}
+
 static inline unsigned char __swp_tb_get_count(unsigned long swp_tb)
 {
 	VM_WARN_ON(!swp_tb_is_countable(swp_tb));
Signature · 1 line
2.54.0
author reviewer
#2 lore raw

RFC PATCH 2/7 mm/zswap: add swp_tb_val to zswap_entry and Pointer accessor helpers

In reply to Baoquan He

Add 'swp_tb_val' to struct zswap_entry to preserve the original swap table entry value when a Shadow or PFN entry is replaced by a Pointer entry.

Add accessor helpers that read and modify swap count and flags through entry->swp_tb_val:
- zswap_swp_tb_get_count(): extract swap count from the saved entry
- zswap_swp_tb_get_flags(): extract flags from the saved entry
- zswap_swp_tb_dup_count(): increment swap count, returns -ENOMEM on overflow
- zswap_swp_tb_put_count(): decrement swap count

The saved swap table entry value uses the Shadow/PFN format, so the existing __swp_tb_get_count/flags and __swp_tb_mk_count helpers work directly on swp_tb_val without any format conversion.

---
 mm/swap.h  | 27 +++++++++++++++++
 mm/zswap.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)
mm/swap.h +27 -0
index 44ab8e1e595b..afe2200bd160 100644
@@ -210,6 +210,33 @@ static inline void swap_cluster_unlock_irq(struct swap_cluster_info *ci)
 
 extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 
+#ifdef CONFIG_ZSWAP
+unsigned char zswap_swp_tb_get_count(unsigned long swp_tb);
+unsigned char zswap_swp_tb_get_flags(unsigned long swp_tb);
+int zswap_swp_tb_dup_count(unsigned long swp_tb,
+			   struct swap_cluster_info *ci, unsigned int ci_off);
+void zswap_swp_tb_put_count(unsigned long swp_tb,
+			    struct swap_cluster_info *ci, unsigned int ci_off);
+#else
+static inline unsigned char zswap_swp_tb_get_count(unsigned long swp_tb)
+{
+	return 0;
+}
+static inline unsigned char zswap_swp_tb_get_flags(unsigned long swp_tb)
+{
+	return 0;
+}
+static inline int zswap_swp_tb_dup_count(unsigned long swp_tb,
+					 struct swap_cluster_info *ci,
+					 unsigned int ci_off)
+{
+	return 0;
+}
+static inline void zswap_swp_tb_put_count(unsigned long swp_tb,
+					  struct swap_cluster_info *ci,
+					  unsigned int ci_off) {}
+#endif
+
 /*
  * Below are the core routines for doing swap for a folio.
  * All helpers requires the folio to be locked, and a locked folio
mm/zswap.c +87 -0

111 lines not on this page · raw message

Signature · 1 line
2.54.0
author reviewer
#3 lore raw

RFC PATCH 3/7 mm/swap_state: do zswap conversion in __swap_cache_do_del_folio

In reply to Baoquan He

Add zswap_try_convert_to_pointer(), called from __swap_cache_do_del_folio under cluster lock. When a folio is evicted from swap cache and a Shadow entry is written, this helper checks if zswap has a compressed copy of the slot and replaces the Shadow with a Pointer entry.

---
 mm/swap.h       |  5 +++++
 mm/swap_state.c |  7 +++++++
 mm/zswap.c      | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
mm/swap.h +5 -0
index afe2200bd160..1b07a08e5277 100644
@@ -211,6 +211,8 @@ static inline void swap_cluster_unlock_irq(struct swap_cluster_info *ci)
 extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 
 #ifdef CONFIG_ZSWAP
+void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
+				  unsigned int ci_off, int type, pgoff_t offset);
 unsigned char zswap_swp_tb_get_count(unsigned long swp_tb);
 unsigned char zswap_swp_tb_get_flags(unsigned long swp_tb);
 int zswap_swp_tb_dup_count(unsigned long swp_tb,
@@ -218,6 +220,9 @@ int zswap_swp_tb_dup_count(unsigned long swp_tb,
 void zswap_swp_tb_put_count(unsigned long swp_tb,
 			    struct swap_cluster_info *ci, unsigned int ci_off);
 #else
+static inline void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
+						unsigned int ci_off, int type,
+						pgoff_t offset) {}
 static inline unsigned char zswap_swp_tb_get_count(unsigned long swp_tb)
 {
 	return 0;
mm/swap_state.c +7 -0
index 6fd6e3415b71..20fb57adbb7f 100644
@@ -278,6 +278,13 @@ static void __swap_cache_do_del_folio(struct swap_cluster_info *ci,
 		/* If shadow is NULL, we set an empty shadow. */
 		__swap_table_set(ci, ci_off, shadow_to_swp_tb(shadow,
 				 __swp_tb_get_flags(old_tb)));
+		/*
+		 * If zswap has a compressed copy of this slot, convert the
+		 * just-written Shadow to a Pointer entry referencing the
+		 * zswap_entry, so the data can be found via the swap table.
+		 */
+		zswap_try_convert_to_pointer(ci, ci_off, si->type,
+					     swp_offset(entry) + ci_off - ci_start);
 	} while (++ci_off < ci_end);
 
 	folio->swap.val = 0;
mm/zswap.c +35 -0
index 81115ca4d10a..f6e8259ab56a 100644
@@ -1652,6 +1652,41 @@ void zswap_invalidate(swp_entry_t swp)
 		zswap_entry_free(entry);
 }
 
+/*
+ * This is called after the Shadow entry has been written. We replace the
+ * just-written Shadow with a Pointer entry pointing to the zswap_entry.
+ *
+ * Context: cluster lock must be held. Cannot sleep.
+ */
+void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
+				  unsigned int ci_off, int type, pgoff_t offset)
+{
+	struct xarray *tree;
+	struct zswap_entry *entry;
+	unsigned long swp_tb;
+
+	tree = &zswap_trees[type][offset >> ZSWAP_ADDRESS_SPACE_SHIFT];
+	if (xa_empty(tree))
+		return;
+
+	entry = xa_load(tree, offset);
+	if (!entry)
+		return;
+
+	swp_tb = __swap_table_get(ci, ci_off);
+	/* Entry must be a Shadow (just written by __swap_cache_do_del_folio) */
+	if (!swp_tb_is_countable(swp_tb))
+		return;
+
+	/*
+	 * Save the complete original swap table entry value, which includes
+	 * the swap count, zero flag, and working set shadow information.
+	 */
+	entry->swp_tb_val = swp_tb;
+
+	__swap_table_set(ci, ci_off, pointer_to_swp_tb(entry));
+}
+
 /*
  * Read the swap count from a Pointer-type swap table entry. The count
  * and flags are stored in zswap_entry->swp_tb_val which preserves the
Signature · 1 line
2.54.0
author reviewer
#4 lore raw

RFC PATCH 4/7 mm/zswap, mm/swap_state: handle Pointer entries in add_folio path

In reply to Baoquan He

Update __swap_cache_add_check and __swap_cache_do_add_folio to handle Pointer-type swap table entries.

Add zswap_swp_tb_get_shadow() helper that extracts the xa_value shadow from the saved swap table entry value in zswap_entry->swp_tb_val.

---
 mm/swap.h       |   5 +++
 mm/swap_state.c | 112 ++++++++++++++++++++++++++++++++++++------------
 mm/zswap.c      |  22 ++++++++++
 3 files changed, 111 insertions(+), 28 deletions(-)
mm/swap.h +5 -0
index 1b07a08e5277..c23f54dc32bd 100644
@@ -213,6 +213,7 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 #ifdef CONFIG_ZSWAP
 void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
 				  unsigned int ci_off, int type, pgoff_t offset);
+void *zswap_swp_tb_get_shadow(unsigned long swp_tb);
 unsigned char zswap_swp_tb_get_count(unsigned long swp_tb);
 unsigned char zswap_swp_tb_get_flags(unsigned long swp_tb);
 int zswap_swp_tb_dup_count(unsigned long swp_tb,
@@ -223,6 +224,10 @@ void zswap_swp_tb_put_count(unsigned long swp_tb,
 static inline void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
 						unsigned int ci_off, int type,
 						pgoff_t offset) {}
+static inline void *zswap_swp_tb_get_shadow(unsigned long swp_tb)
+{
+	return NULL;
+}
 static inline unsigned char zswap_swp_tb_get_count(unsigned long swp_tb)
 {
 	return 0;
mm/swap_state.c +84 -28

220 lines not on this page · raw message

mm/zswap.c +22 -0
index f6e8259ab56a..1bb484d1ac87 100644
@@ -1687,6 +1687,28 @@ void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
 	__swap_table_set(ci, ci_off, pointer_to_swp_tb(entry));
 }
 
+/*
+ * Extract the working set shadow from a Pointer-type swap table entry.
+ * The shadow is recovered from zswap_entry->swp_tb_val which preserves
+ * the original Shadow entry value. The shadow can then be used by
+ * workingset_refault() for refault distance calculation.
+ */
+void *zswap_swp_tb_get_shadow(unsigned long swp_tb)
+{
+	struct zswap_entry *entry = swp_tb_to_pointer(swp_tb);
+	unsigned long saved = entry->swp_tb_val;
+
+	/*
+	 * swp_tb_val preserves the original swap table entry.  Only
+	 * Shadow entries carry workingset information; PFN and NULL
+	 * entries have no shadow to extract.
+	 */
+	if (!swp_tb_is_shadow(saved))
+		return NULL;
+
+	return (void *)(saved & ~SWP_TB_FLAGS_MASK);
+}
+
 /*
  * Read the swap count from a Pointer-type swap table entry. The count
  * and flags are stored in zswap_entry->swp_tb_val which preserves the
Signature · 1 line
2.54.0
author reviewer
#5 lore raw

RFC PATCH 5/7 mm/swapfile: handle Pointer entries in count/dup/put paths

In reply to Baoquan He

Update swap count, duplicate, and put operations to handle Pointer-type swap table entries where the count is stored in zswap_entry->swp_tb_val rather than inline in the entry bits.

---
 mm/swapfile.c | 95 ++++++++++++++++++++++++++++++++++++++++-----------
 mm/zswap.c    |  5 +++
 2 files changed, 81 insertions(+), 19 deletions(-)
mm/swapfile.c +76 -19
index a602e5820513..15572383cafa 100644
@@ -1536,25 +1536,51 @@ static void __swap_cluster_put_entry(struct swap_cluster_info *ci,
 
 	lockdep_assert_held(&ci->lock);
 	swp_tb = __swap_table_get(ci, ci_off);
+
+	/* Pointer entries store count in the zswap_entry struct */
+	if (swp_tb_is_pointer(swp_tb)) {
+		zswap_swp_tb_put_count(swp_tb, ci, ci_off);
+		return;
+	}
+
 	count = __swp_tb_get_count(swp_tb);
 
 	VM_WARN_ON_ONCE(count <= 0);
 	VM_WARN_ON_ONCE(count > SWP_TB_COUNT_MAX);
-
-	if (count == SWP_TB_COUNT_MAX) {
-		count = ci->extend_table[ci_off];
-		/* Overflow starts with SWP_TB_COUNT_MAX */
-		VM_WARN_ON_ONCE(count < SWP_TB_COUNT_MAX);
-		count--;
-		if (count == (SWP_TB_COUNT_MAX - 1)) {
-			ci->extend_table[ci_off] = 0;
-			__swap_table_set(ci, ci_off, __swp_tb_mk_count(swp_tb, count));
+		if (count == SWP_TB_COUNT_MAX) {
+			if (!ci->extend_table) {
+				/*
+				 * MAX without extend_table: MAX is the real
+				 * count.  Decrement and write back inline.
+				 */
+				count--;
+				__swap_table_set(ci, ci_off,
+					__swp_tb_mk_count(swp_tb, count));
+			} else {
+				/*
+				 * MAX with extend_table: real count lives in
+				 * ci->extend_table[ci_off].  Pull, decrement
+				 * it, and either move it back inline (if now
+				 * fits) or write it back to the extend table.
+				 */
+				count = ci->extend_table[ci_off];
+				/* Overflow starts with SWP_TB_COUNT_MAX */
+				VM_WARN_ON_ONCE(count < SWP_TB_COUNT_MAX);
+				count--;
+				if (count == (SWP_TB_COUNT_MAX - 1)) {
+					ci->extend_table[ci_off] = 0;
+					__swap_table_set(ci, ci_off,
+						__swp_tb_mk_count(swp_tb, count));
+				} else {
+					ci->extend_table[ci_off] = count;
+				}
+			}
 		} else {
-			ci->extend_table[ci_off] = count;
+			/* count < MAX: simple inline decrement */
+			count--;
+			__swap_table_set(ci, ci_off,
+				__swp_tb_mk_count(swp_tb, count));
 		}
-	} else {
-		__swap_table_set(ci, ci_off, __swp_tb_mk_count(swp_tb, --count));
-	}
 
 	/*
 	 * `SWP_TB_COUNT_MAX - 1` triggers extend table allocation. If the
@@ -1596,7 +1622,18 @@ static void swap_put_entries_cluster(struct swap_info_struct *si,
 	ci_end = ci_off + nr;
 	do {
 		swp_tb = __swap_table_get(ci, ci_off);
-		if (swp_tb_get_count(swp_tb) == 1) {
+		/*
+		 * Pointer entries store count in the zswap_entry;
+		 * handle them before swp_tb_get_count (which returns
+		 * -EINVAL for non-countable types).
+		 */
+		if (swp_tb_is_pointer(swp_tb)) {
+			if (zswap_swp_tb_get_count(swp_tb) == 1) {
+				if (ci_batch == -1)
+					ci_batch = ci_off;
+				continue;
+			}
+		} else if (swp_tb_get_count(swp_tb) == 1) {
 			/* count == 1 and non-cached slots will be batch freed. */
 			if (!swp_tb_is_folio(swp_tb)) {
 				if (ci_batch == -1)
@@ -1647,6 +1684,11 @@ static int __swap_cluster_dup_entry(struct swap_cluster_info *ci,
 	/* Bad or special slots can't be handled */
 	if (WARN_ON_ONCE(swp_tb_is_bad(swp_tb)))
 		return -EINVAL;
+
+	/* Pointer entries store count in the zswap_entry struct */
+	if (swp_tb_is_pointer(swp_tb))
+		return zswap_swp_tb_dup_count(swp_tb, ci, ci_off);
+
 	count = __swp_tb_get_count(swp_tb);
 	/* Must be either cached or have a count already */
 	if (WARN_ON_ONCE(!count && !swp_tb_is_folio(swp_tb)))
@@ -1923,13 +1965,19 @@ void __swap_cluster_free_entries(struct swap_info_struct *si,
 		old_tb = __swap_table_get(ci, ci_off);
 		/*
 		 * Freeing is done after release of the last swap count
-		 * ref, or after swap cache is dropped
+		 * ref, or after swap cache is dropped. A Pointer entry
+		 * means zswap has a compressed copy; the xarray still
+		 * holds the entry and zswap_invalidate will free it.
 		 */
-		VM_WARN_ON(!swp_tb_is_shadow(old_tb) || __swp_tb_get_count(old_tb) > 1);
+		if (swp_tb_is_pointer(old_tb))
+			VM_WARN_ON(zswap_swp_tb_get_count(old_tb) > 1);
+		else
+			VM_WARN_ON(!swp_tb_is_shadow(old_tb) ||
+				   __swp_tb_get_count(old_tb) > 1);
 
 		/* Resetting the slot to NULL also clears the inline flags. */
 		__swap_table_set(ci, ci_off, null_to_swp_tb());
-		if (!SWAP_TABLE_HAS_ZEROFLAG)
+		if (!SWAP_TABLE_HAS_ZEROFLAG && !swp_tb_is_pointer(old_tb))
 			__swap_table_clear_zero(ci, ci_off);
 
 		/*
@@ -1961,8 +2009,11 @@ int __swap_count(swp_entry_t entry)
 {
 	struct swap_cluster_info *ci = __swap_entry_to_cluster(entry);
 	unsigned int ci_off = swp_cluster_offset(entry);
+	unsigned long swp_tb = __swap_table_get(ci, ci_off);
 
-	return swp_tb_get_count(__swap_table_get(ci, ci_off));
+	if (swp_tb_is_pointer(swp_tb))
+		return zswap_swp_tb_get_count(swp_tb);
+	return swp_tb_get_count(swp_tb);
 }
 
 /**
@@ -1980,6 +2031,8 @@ bool swap_entry_swapped(struct swap_info_struct *si, swp_entry_t entry)
 	swp_tb = swap_table_get(ci, offset % SWAPFILE_CLUSTER);
 	swap_cluster_unlock(ci);
 
+	if (swp_tb_is_pointer(swp_tb))
+		return zswap_swp_tb_get_count(swp_tb) > 0;
 	return swp_tb_get_count(swp_tb) > 0;
 }
 
@@ -2556,8 +2609,12 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 		if (!folio) {
 			swp_tb = swap_table_get(__swap_entry_to_cluster(entry),
 						swp_cluster_offset(entry));
-			if (swp_tb_get_count(swp_tb) <= 0)
+			if (swp_tb_is_pointer(swp_tb)) {
+				if (zswap_swp_tb_get_count(swp_tb) <= 0)
+					continue;
+			} else if (swp_tb_get_count(swp_tb) <= 0) {
 				continue;
+			}
 			return -ENOMEM;
 		}
 
mm/zswap.c +5 -0
index 1bb484d1ac87..58259673ce2b 100644
@@ -1774,6 +1774,11 @@ void zswap_swp_tb_put_count(unsigned long swp_tb,
 	VM_WARN_ON_ONCE(count == 0);
 
 	if (count == SWP_TB_COUNT_MAX) {
+		if (!ci->extend_table) {
+			count--;
+			entry->swp_tb_val = __swp_tb_mk_count(entry->swp_tb_val, count);
+			return;
+		}
 		count = ci->extend_table[ci_off];
 		/* Overflow starts with SWP_TB_COUNT_MAX */
 		VM_WARN_ON_ONCE(count < SWP_TB_COUNT_MAX);
Signature · 1 line
2.54.0
author reviewer
#6 lore raw

RFC PATCH 6/7 mm/zswap, mm/swap_state: migrate zswap to use swap_table for entry indexing

In reply to Baoquan He

Transition zswap to use swap table entries as the primary index for locating compressed data. Key changes:

---
 mm/swap_state.c | 62 ++++++++++++++---------------------
 mm/zswap.c      | 87 ++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 97 insertions(+), 52 deletions(-)
mm/swap_state.c +25 -37
index 620dc763eb3b..b3d5bedfaf07 100644
@@ -214,8 +214,7 @@ static int __swap_cache_add_check(struct swap_cluster_info *ci,
 }
 
 static void __swap_cache_do_add_folio(struct swap_cluster_info *ci,
-				      struct folio *folio, swp_entry_t entry,
-				      void **zentry)
+				      struct folio *folio, swp_entry_t entry)
 {
 	unsigned int ci_off = swp_cluster_offset(entry), ci_end;
 	unsigned long nr_pages = folio_nr_pages(folio);
@@ -232,24 +231,15 @@ static void __swap_cache_do_add_folio(struct swap_cluster_info *ci,
 		VM_WARN_ON_ONCE(swp_tb_is_folio(old_tb));
 		/*
 		 * If the old entry is a Pointer (zswap compressed data),
-		 * extract the zswap_entry for the caller and preserve
-		 * its flags. The Pointer entry has no inline flags.
+		 * extract the flags from the zswap_entry. The Pointer
+		 * entry has no inline flags.
 		 */
-		if (swp_tb_is_pointer(old_tb)) {
-			unsigned char cur_count, cur_flags;
-			unsigned long new_tb;
-
-			cur_count = zswap_swp_tb_get_count(old_tb);
-			cur_flags = zswap_swp_tb_get_flags(old_tb);
-			if (zentry && !*zentry)
-				*zentry = swp_tb_to_pointer(old_tb);
-			new_tb = pfn_to_swp_tb(pfn, cur_flags);
-			new_tb = __swp_tb_mk_count(new_tb, cur_count);
-			__swap_table_set(ci, ci_off, new_tb);
-		} else {
+		if (swp_tb_is_pointer(old_tb))
+			__swap_table_set(ci, ci_off,
+				pfn_to_swp_tb(pfn, zswap_swp_tb_get_flags(old_tb)));
+		else
 			__swap_table_set(ci, ci_off,
 				pfn_to_swp_tb(pfn, __swp_tb_get_flags(old_tb)));
-		}
 	} while (++ci_off < ci_end);
 
 	folio_ref_add(folio, nr_pages);
@@ -272,12 +262,11 @@ static void __swap_cache_do_add_folio(struct swap_cluster_info *ci,
  * that holds the entries.
  */
 void __swap_cache_add_folio(struct swap_cluster_info *ci,
-			    struct folio *folio, swp_entry_t entry,
-			    void **zentry)
+			    struct folio *folio, swp_entry_t entry)
 {
 	unsigned long nr_pages = folio_nr_pages(folio);
 
-	__swap_cache_do_add_folio(ci, folio, entry, zentry);
+	__swap_cache_do_add_folio(ci, folio, entry);
 	node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
 	lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
 }
@@ -324,6 +313,14 @@ static void __swap_cache_do_del_folio(struct swap_cluster_info *ci,
 			/* If shadow is NULL, we set an empty shadow. */
 			__swap_table_set(ci, ci_off, shadow_to_swp_tb(shadow,
 					 __swp_tb_get_flags(old_tb)));
+			/*
+			 * If zswap has a compressed copy of this slot,
+			 * convert the just-written Shadow to a Pointer
+			 * entry referencing the zswap_entry.
+			 */
+			zswap_try_convert_to_pointer(ci, ci_off, si->type,
+						     swp_offset(entry) +
+						     ci_off - ci_start);
 		}
 	} while (++ci_off < ci_end);
 
@@ -457,8 +454,7 @@ void __swap_cache_replace_folio(struct swap_cluster_info *ci,
 static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
 					swp_entry_t targ_entry, gfp_t gfp,
 					unsigned int order, struct vm_fault *vmf,
-					struct mempolicy *mpol, pgoff_t ilx,
-					void **zentry)
+					struct mempolicy *mpol, pgoff_t ilx)
 {
 	int err;
 	swp_entry_t entry;
@@ -505,7 +501,7 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
 
 	__folio_set_locked(folio);
 	__folio_set_swapbacked(folio);
-	__swap_cache_do_add_folio(ci, folio, entry, zentry);
+	__swap_cache_do_add_folio(ci, folio, entry);
 	spin_unlock(&ci->lock);
 
 	if (mem_cgroup_swapin_charge_folio(folio, memcg_id,
@@ -562,8 +558,7 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
  */
 struct folio *swap_cache_alloc_folio(swp_entry_t targ_entry, gfp_t gfp,
 				     unsigned long orders, struct vm_fault *vmf,
-				     struct mempolicy *mpol, pgoff_t ilx,
-				     void **zentry)
+				     struct mempolicy *mpol, pgoff_t ilx)
 {
 	int order, err;
 	struct folio *ret;
@@ -576,12 +571,9 @@ struct folio *swap_cache_alloc_folio(swp_entry_t targ_entry, gfp_t gfp,
 	if (WARN_ON_ONCE(!orders || (1UL << order) > SWAPFILE_CLUSTER))
 		return ERR_PTR(-EINVAL);
 
-	if (zentry)
-		*zentry = NULL;
-
 	do {
 		ret = __swap_cache_alloc(ci, targ_entry, gfp, order,
-					 vmf, mpol, ilx, zentry);
+					 vmf, mpol, ilx);
 		if (!IS_ERR(ret))
 			break;
 		err = PTR_ERR(ret);
@@ -697,20 +689,18 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
 					   struct swap_iocb **plug, bool readahead)
 {
 	struct folio *folio;
-	void *zentry;
 
 	do {
 		folio = swap_cache_get_folio(entry);
 		if (folio)
 			return folio;
-		folio = swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx,
-					       &zentry);
+		folio = swap_cache_alloc_folio(entry, gfp, BIT(0), NULL, mpol, ilx);
 	} while (PTR_ERR(folio) == -EEXIST);
 
 	if (IS_ERR_OR_NULL(folio))
 		return NULL;
 
-	swap_read_folio(folio, plug, zentry);
+	swap_read_folio(folio, plug);
 	if (readahead) {
 		folio_set_readahead(folio);
 		count_vm_event(SWAP_RA);
@@ -739,20 +729,18 @@ struct folio *swapin_sync(swp_entry_t entry, gfp_t gfp, unsigned long orders,
 			   struct vm_fault *vmf, struct mempolicy *mpol, pgoff_t ilx)
 {
 	struct folio *folio;
-	void *zentry;
 
 	do {
 		folio = swap_cache_get_folio(entry);
 		if (folio)
 			return folio;
-		folio = swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx,
-					       &zentry);
+		folio = swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx);
 	} while (PTR_ERR(folio) == -EEXIST);
 
 	if (IS_ERR(folio))
 		return folio;
 
-	swap_read_folio(folio, NULL, zentry);
+	swap_read_folio(folio, NULL);
 	return folio;
 }
 
mm/zswap.c +72 -15

133 lines not on this page · raw message

Signature · 1 line
2.54.0
author reviewer
#7 lore raw

RFC PATCH 7/7 mm/zswap, mm/swap_state: replace xarray indexing with swap table lookups

In reply to Baoquan He

Remove the xarray-based zswap entry lookup infrastructure entirely and use the swap table Pointer entries as the sole index for locating compressed zswap data.

Thread the zswap_entry through the swapin call chain via a new void **zentry parameter.

Update all declarations in swap.h and include/linux/zswap.h to match the new signatures.

---
 include/linux/zswap.h |   4 +-
 mm/page_io.c          |   4 +-
 mm/swap.h             |  25 +++--
 mm/swap_state.c       |  68 ++++++++----
 mm/swapfile.c         |  16 ++-
 mm/zswap.c            | 252 +++++++++++++++---------------------------
 6 files changed, 165 insertions(+), 204 deletions(-)
include/linux/zswap.h +2 -2
index 30c193a1207e..4f55c8d0c4c5 100644
@@ -26,7 +26,7 @@ struct zswap_lruvec_state {
 
 unsigned long zswap_total_pages(void);
 bool zswap_store(struct folio *folio);
-int zswap_load(struct folio *folio);
+int zswap_load(struct folio *folio, void *zentry);
 void zswap_invalidate(swp_entry_t swp);
 int zswap_swapon(int type, unsigned long nr_pages);
 void zswap_swapoff(int type);
@@ -44,7 +44,7 @@ static inline bool zswap_store(struct folio *folio)
 	return false;
 }
 
-static inline int zswap_load(struct folio *folio)
+static inline int zswap_load(struct folio *folio, void *zentry)
 {
 	return -ENOENT;
 }
mm/page_io.c +2 -2
index b23f494fcc83..1ce8201ed86e 100644
@@ -658,7 +658,7 @@ static void swap_read_folio_bdev_async(struct folio *folio,
 	submit_bio(bio);
 }
 
-void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
+void swap_read_folio(struct folio *folio, struct swap_iocb **plug, void *zentry)
 {
 	struct swap_info_struct *sis = __swap_entry_to_info(folio->swap);
 	bool synchronous = sis->flags & SWP_SYNCHRONOUS_IO;
@@ -686,7 +686,7 @@ void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
 		goto finish;
 	}
 
-	if (zswap_load(folio) != -ENOENT)
+	if (zswap_load(folio, zentry) != -ENOENT)
 		goto finish;
 
 	/* We have to read from slower devices. Increase zswap protection. */
mm/swap.h +16 -9
index c23f54dc32bd..84e13ba6e605 100644
@@ -210,9 +210,8 @@ static inline void swap_cluster_unlock_irq(struct swap_cluster_info *ci)
 
 extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 
+struct zswap_entry;
 #ifdef CONFIG_ZSWAP
-void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
-				  unsigned int ci_off, int type, pgoff_t offset);
 void *zswap_swp_tb_get_shadow(unsigned long swp_tb);
 unsigned char zswap_swp_tb_get_count(unsigned long swp_tb);
 unsigned char zswap_swp_tb_get_flags(unsigned long swp_tb);
@@ -220,10 +219,8 @@ int zswap_swp_tb_dup_count(unsigned long swp_tb,
 			   struct swap_cluster_info *ci, unsigned int ci_off);
 void zswap_swp_tb_put_count(unsigned long swp_tb,
 			    struct swap_cluster_info *ci, unsigned int ci_off);
+void zswap_entry_free(struct zswap_entry *entry);
 #else
-static inline void zswap_try_convert_to_pointer(struct swap_cluster_info *ci,
-						unsigned int ci_off, int type,
-						pgoff_t offset) {}
 static inline void *zswap_swp_tb_get_shadow(unsigned long swp_tb)
 {
 	return NULL;
@@ -245,6 +242,7 @@ static inline int zswap_swp_tb_dup_count(unsigned long swp_tb,
 static inline void zswap_swp_tb_put_count(unsigned long swp_tb,
 					  struct swap_cluster_info *ci,
 					  unsigned int ci_off) {}
+static inline void zswap_entry_free(struct zswap_entry *entry) {}
 #endif
 
 /*
@@ -278,7 +276,7 @@ extern void __swap_cluster_free_entries(struct swap_info_struct *si,
 /* linux/mm/page_io.c */
 int sio_pool_init(void);
 struct swap_iocb;
-void swap_read_folio(struct folio *folio, struct swap_iocb **plug);
+void swap_read_folio(struct folio *folio, struct swap_iocb **plug, void *zentry);
 void __swap_read_unplug(struct swap_iocb *plug);
 static inline void swap_read_unplug(struct swap_iocb *plug)
 {
@@ -340,13 +338,16 @@ static inline bool folio_matches_swap_entry(const struct folio *folio,
 bool swap_cache_has_folio(swp_entry_t entry);
 struct folio *swap_cache_get_folio(swp_entry_t entry);
 void *swap_cache_get_shadow(swp_entry_t entry);
+bool folio_maybe_swapped(struct folio *folio);
 void swap_cache_del_folio(struct folio *folio);
 struct folio *swap_cache_alloc_folio(swp_entry_t target_entry, gfp_t gfp_mask,
 				     unsigned long orders, struct vm_fault *vmf,
-				     struct mempolicy *mpol, pgoff_t ilx);
+				     struct mempolicy *mpol, pgoff_t ilx,
+				     void **zentry);
 /* Below helpers require the caller to lock and pass in the swap cluster. */
 void __swap_cache_add_folio(struct swap_cluster_info *ci,
-			    struct folio *folio, swp_entry_t entry);
+			    struct folio *folio, swp_entry_t entry,
+			    void **zentry);
 void __swap_cache_del_folio(struct swap_cluster_info *ci,
 			    struct folio *folio, swp_entry_t entry, void *shadow);
 void __swap_cache_replace_folio(struct swap_cluster_info *ci,
@@ -418,7 +419,8 @@ static inline void folio_put_swap(struct folio *folio, struct page *page)
 {
 }
 
-static inline void swap_read_folio(struct folio *folio, struct swap_iocb **plug)
+static inline void swap_read_folio(struct folio *folio, struct swap_iocb **plug,
+				   void *zentry)
 {
 }
 
@@ -490,6 +492,11 @@ static inline void *swap_cache_get_shadow(swp_entry_t entry)
 	return NULL;
 }
 
+static inline bool folio_maybe_swapped(struct folio *folio)
+{
+	return false;
+}
+
 static inline void swap_cache_del_folio(struct folio *folio)
 {
 }
mm/swap_state.c +44 -24

157 lines not on this page · raw message

mm/swapfile.c +12 -4
index 15572383cafa..18925e462571 100644
@@ -935,7 +935,8 @@ static bool __swap_cluster_alloc_entries(struct swap_info_struct *si,
 		nr_pages = 1 << order;
 		swap_cluster_assert_empty(ci, ci_off, nr_pages, false);
 		__swap_cache_add_folio(ci, folio, swp_entry(si->type,
-							    ci_off + cluster_offset(si, ci)));
+							    ci_off + cluster_offset(si, ci)),
+				       NULL);
 	} else if (IS_ENABLED(CONFIG_HIBERNATION)) {
 		order = 0;
 		nr_pages = 1;
@@ -1969,11 +1970,18 @@ void __swap_cluster_free_entries(struct swap_info_struct *si,
 		 * means zswap has a compressed copy; the xarray still
 		 * holds the entry and zswap_invalidate will free it.
 		 */
-		if (swp_tb_is_pointer(old_tb))
+		if (swp_tb_is_pointer(old_tb)) {
 			VM_WARN_ON(zswap_swp_tb_get_count(old_tb) > 1);
-		else
+			/*
+			 * Free the zswap entry now under ci->lock while
+			 * the Pointer is still in the swap table.  Once
+			 * we null the slot, the entry would be leaked.
+			 */
+			zswap_entry_free(swp_tb_to_pointer(old_tb));
+		} else {
 			VM_WARN_ON(!swp_tb_is_shadow(old_tb) ||
 				   __swp_tb_get_count(old_tb) > 1);
+		}
 
 		/* Resetting the slot to NULL also clears the inline flags. */
 		__swap_table_set(ci, ci_off, null_to_swp_tb());
@@ -2073,7 +2081,7 @@ int swp_swapcount(swp_entry_t entry)
  *
  * Context: Caller must ensure the folio is locked and in the swap cache.
  */
-static bool folio_maybe_swapped(struct folio *folio)
+bool folio_maybe_swapped(struct folio *folio)
 {
 	swp_entry_t entry = folio->swap;
 	struct swap_cluster_info *ci;
mm/zswap.c +89 -163

400 lines not on this page · raw message

Signature · 1 line
2.54.0
maintainer
#8 lore raw
In reply to Baoquan He
Quoted 10 lines · Baoquan He

Currently zswap uses a per-swap-device xarray to map swap offsets to zswap_entry pointers. This introduces an additional tree walk on every zswap entry access.

This series replaces the xarray with a "Pointer" entry type stored directly in the swap cluster table. The new swp_tb_t variant embeds a zswap_entry pointer plus a entry count, eliminating the xarray and its lookup overhead entirely.

Have you had the chance to review the virtual swap work (latest version in [1])? At a first glance, it bears a lot of similarity to this.

We're envisioning this to be the future architecture for zswap :) Is there anything that is missing that would satisfy your use case too?

[1]: https://lore.kernel.org/all/[email protected]/

maintainer
#9 lore raw
Quoted 13 lines · Nhat Pham
Quoted 10 lines · Baoquan He

Currently zswap uses a per-swap-device xarray to map swap offsets to zswap_entry pointers. This introduces an additional tree walk on every zswap entry access.

This series replaces the xarray with a "Pointer" entry type stored directly in the swap cluster table. The new swp_tb_t variant embeds a zswap_entry pointer plus a entry count, eliminating the xarray and its lookup overhead entirely.

Quoted 8 lines

Have you had the chance to review the virtual swap work (latest version in [1])? At a first glance, it bears a lot of similarity to this.

We're envisioning this to be the future architecture for zswap :) Is there anything that is missing that would satisfy your use case too?

[1]: https://lore.kernel.org/all/[email protected]/

Yeah this doubles down on the dependency of zswap on a swapfile being present, but I guess the goal is for this to be complimented by your other series to add ghost swapfile support. This was discussed at length, and we don't really want to have a user-facing ghost/virtual swapfile concept, at least not for now. Nhat's latest proposal is essentially a ghost/virtual swapfile implementation that is transparent to userspace, AFAICT.

As Nhat mentioned, if there are deficiencies in his proposal that are addressed by yours, I think the most productive way forward is to start a discussion (here or on Nhat's proposal) to figure out the best way forward for everyone.

maintainer
#10 lore raw
Quoted 32 lines · Yosry Ahmed
Quoted 13 lines · Nhat Pham
Quoted 10 lines · Baoquan He

Currently zswap uses a per-swap-device xarray to map swap offsets to zswap_entry pointers. This introduces an additional tree walk on every zswap entry access.

This series replaces the xarray with a "Pointer" entry type stored directly in the swap cluster table. The new swp_tb_t variant embeds a zswap_entry pointer plus a entry count, eliminating the xarray and its lookup overhead entirely.

Quoted 8 lines

Have you had the chance to review the virtual swap work (latest version in [1])? At a first glance, it bears a lot of similarity to this.

We're envisioning this to be the future architecture for zswap :) Is there anything that is missing that would satisfy your use case too?

[1]: https://lore.kernel.org/all/[email protected]/

Yeah this doubles down on the dependency of zswap on a swapfile being present, but I guess the goal is for this to be complimented by your other series to add ghost swapfile support. This was discussed at length, and we don't really want to have a user-facing ghost/virtual swapfile concept, at least not for now. Nhat's latest proposal is essentially a ghost/virtual swapfile implementation that is transparent to userspace, AFAICT.

Quoted 4 lines

As Nhat mentioned, if there are deficiencies in his proposal that are addressed by yours, I think the most productive way forward is to start a discussion (here or on Nhat's proposal) to figure out the best way forward for everyone.

Now that I have taken another look at the other series too, I realized that these 2 are really a big patch series, split into two. Vswap also merges the zswap tree into the swap table infrastructure, and the code paths are actually *simpler*: a lot of the swap-cache/swap refcounting logic remains unchanged, whether the swap entry mapped to PTE belongs to a virtual or physical swapfile.

With this new design:

1. Metadata can now live in struct zswap entry, if the swapfile is zswap ghost swapfile.

2. You need to move the metadata around if you need to do zswap writeback.

It seems rather misleading to claim that the ghost zswap swapfile design is simpler, when a lot of the complexity involved in this layer merging is hidden in this optimization patch series.

If you're concerned about the xarray overhead, I'm willing to consider the vmalloc array data structure. You can then graft the other optimization in this series on to the vswap work.

syzbot Tested-by [email protected] on the v1 series
#11 lore raw
In reply to Baoquan He
syzbot ci has tested the following series

syzbot ci has tested the following series

[v1] mm: store zswap entries in swap table (Pointer entry)
https://lore.kernel.org/all/[email protected]
* [RFC PATCH 1/7] mm/swap_table: add Pointer type support for zswap entry storage
* [RFC PATCH 2/7] mm/zswap: add swp_tb_val to zswap_entry and Pointer accessor helpers
* [RFC PATCH 3/7] mm/swap_state: do zswap conversion in __swap_cache_do_del_folio
* [RFC PATCH 4/7] mm/zswap, mm/swap_state: handle Pointer entries in add_folio path
* [RFC PATCH 5/7] mm/swapfile: handle Pointer entries in count/dup/put paths
* [RFC PATCH 6/7] mm/zswap, mm/swap_state: migrate zswap to use swap_table for entry indexing
* [RFC PATCH 7/7] mm/zswap, mm/swap_state: replace xarray indexing with swap table lookups

and found the following issue:
WARNING in __swap_cache_do_del_folio

Full report is available here:
https://ci.syzbot.org/series/1a9cfadb-ed85-48c1-b1b3-a3c12ea99471

***

WARNING in __swap_cache_do_del_folio

tree:      mm-new
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/akpm/mm.git
base:      01bc0932614c2d3c35e732f54d249b5897d94307
arch:      amd64
compiler:  Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6
config:    https://ci.syzbot.org/builds/65f10583-d539-4b33-8ba9-494d42eb9593/config
syz repro: https://ci.syzbot.org/findings/3673912c-5490-4f26-ba24-276021c43d9b/syz_repro
------------[ cut here ]------------
!swp_tb_is_folio(old_tb) || swp_tb_to_folio(old_tb) != folio
WARNING: mm/swap_state.c:327 at __swap_cache_do_del_folio+0x6ef/0xdf0 mm/swap_state.c:326, CPU#1: syz.2.63/6057
Modules linked in:
CPU: 1 UID: 0 PID: 6057 Comm: syz.2.63 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:__swap_cache_do_del_folio+0x6ef/0xdf0 mm/swap_state.c:326
Code: ff c9 48 09 c1 4c 21 f9 48 39 4c 24 08 0f 85 8c 02 00 00 e8 73 9a 9b ff 49 bf 00 00 00 00 00 fc ff df eb 3d e8 62 9a 9b ff 90 <0f> 0b 90 41 89 ed 41 83 e5 01 31 ff 4c 89 ee e8 2d 9f 9b ff 31 ff
RSP: 0018:ffffc90003ea6690 EFLAGS: 00010093
RAX: ffffffff822acdb9 RBX: 000000000000001f RCX: ffff88816d7d9e00
RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffffea0006be5f08
RBP: 00000000006be5f2 R08: ffff88801c7d20ff R09: 1ffff110038fa41f
R10: dffffc0000000000 R11: ffffed10038fa420 R12: 0000000000000000
R13: 0000000006be5f00 R14: 0000000000000002 R15: dffffc0000000000
FS:  00007fef4622c6c0(0000) GS:ffff8882a921d000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000000280 CR3: 0000000176c44000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 __swap_cache_del_folio+0xd5/0x170 mm/swap_state.c:377
 __remove_mapping+0xabb/0xe20 mm/vmscan.c:736
 shrink_folio_list+0x2b42/0x5350 mm/vmscan.c:1489
 reclaim_folio_list+0x100/0x430 mm/vmscan.c:2163
 reclaim_pages+0x2f1/0x530 mm/vmscan.c:2196
 madvise_cold_or_pageout_pte_range+0x1c1e/0x22e0 mm/madvise.c:562
 walk_pmd_range mm/pagewalk.c:148 [inline]
 walk_pud_range mm/pagewalk.c:239 [inline]
 walk_p4d_range mm/pagewalk.c:280 [inline]
 walk_pgd_range+0x100f/0x1e40 mm/pagewalk.c:321
 __walk_page_range+0x16d/0x720 mm/pagewalk.c:429
 walk_page_range_vma_unsafe+0x309/0x410 mm/pagewalk.c:733
 madvise_pageout_page_range mm/madvise.c:621 [inline]
 madvise_pageout mm/madvise.c:646 [inline]
 madvise_vma_behavior+0x2922/0x4340 mm/madvise.c:1357
 madvise_walk_vmas+0x576/0xb00 mm/madvise.c:1712
 madvise_do_behavior+0x385/0x540 mm/madvise.c:1907
 do_madvise+0x327/0x3a0 mm/madvise.c:2005
 __do_sys_madvise mm/madvise.c:2014 [inline]
 __se_sys_madvise mm/madvise.c:2012 [inline]
 __x64_sys_madvise+0xa6/0xc0 mm/madvise.c:2012
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fef4539ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fef4622c028 EFLAGS: 00000246 ORIG_RAX: 000000000000001c
RAX: ffffffffffffffda RBX: 00007fef45616180 RCX: 00007fef4539ce59
RDX: 0000000000000015 RSI: 0000000000600000 RDI: 0000200000000000
RBP: 00007fef45432e6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fef45616218 R14: 00007fef45616180 R15: 00007ffcd47ce068
 </TASK>

***

If these findings have caused you to resend the series or submit a separate fix, please add the following tag to your commit message:

---
This report is generated by a bot. It may contain errors. syzbot ci engineers can be reached at [email protected].

To test a patch for this bug, please reply with #syz test (should be on a separate line).

The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.

maintainer
#12 lore raw
In reply to Baoquan He
Quoted 10 lines · Baoquan He

Currently zswap uses a per-swap-device xarray to map swap offsets to zswap_entry pointers. This introduces an additional tree walk on every zswap entry access.

This series replaces the xarray with a "Pointer" entry type stored directly in the swap cluster table. The new swp_tb_t variant embeds a zswap_entry pointer plus a entry count, eliminating the xarray and its lookup overhead entirely.

Taking a step back from the ghost swapfile vs vswap debate - for this optimization specifically, couldn't you adopt part of the vswap design?

Specifically, expand the struct swap_cluster_struct to include the virtual_table (feel free to rename it - but please make it generic, like backend or sth, so that vswap can re-use it).

You can now do the rest of your optimizations exactly the same. No need to add new swap table entry type. No need to juggle the swap metadata between swap table and zswap entry's embedded field, which means you don't have to change swap cache and swap reference counting operations.

Now, yes, overhead is a problem indeed. But you can get around it by:

1. Guard the virtual_table array behind CONFIG_ZSWAP.

2. Make this array dynamically allocated. This is crucial, because there will be setups where certain cgroups use zswap, and certain cgroups use their own dedicated fast swapfile backends (via swap tiers), and we do not want to burden the latter with zswap-induced memory overhead. But should be trivial, and we can start with something as simple as allocated on first zswap_store(), or during swap allocation, if the folio's cgroup may zswap, etc.

Obviously, I'm biased, but I *think* this is actually simpler for your optimization approach, no?