diff mbox series

[1/2] mm/zswap: make sure each swapfile always have zswap rb-tree

Message ID 20240117-b4-zswap-lock-optimize-v1-1-23f6effe5775@bytedance.com (mailing list archive)
State New
Headers show
Series mm/zswap: optimize the scalability of zswap rb-tree | expand

Commit Message

Chengming Zhou Jan. 17, 2024, 9:23 a.m. UTC 
Not all zswap interfaces can handle the absence of the zswap rb-tree,
actually only zswap_store() has handled it for now.

To make things simple, we make sure each swapfile always have the
zswap rb-tree prepared before being enabled and used. The preparation
is unlikely to fail in practice, this patch just make it explicit.

Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
 include/linux/zswap.h |  7 +++++--
 mm/swapfile.c         | 10 +++++++---
 mm/zswap.c            |  7 ++++---
 3 files changed, 16 insertions(+), 8 deletions(-)

Comments

Johannes Weiner Jan. 18, 2024, 3:05 p.m. UTC | #1 
On Wed, Jan 17, 2024 at 09:23:18AM +0000, Chengming Zhou wrote:
> Not all zswap interfaces can handle the absence of the zswap rb-tree,
> actually only zswap_store() has handled it for now.
> 
> To make things simple, we make sure each swapfile always have the
> zswap rb-tree prepared before being enabled and used. The preparation
> is unlikely to fail in practice, this patch just make it explicit.
> 
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Yosry Ahmed Jan. 18, 2024, 5:37 p.m. UTC | #2 
On Wed, Jan 17, 2024 at 1:23 AM Chengming Zhou
<zhouchengming@bytedance.com> wrote:
>
> Not all zswap interfaces can handle the absence of the zswap rb-tree,
> actually only zswap_store() has handled it for now.
>
> To make things simple, we make sure each swapfile always have the
> zswap rb-tree prepared before being enabled and used. The preparation
> is unlikely to fail in practice, this patch just make it explicit.
>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

Acked-by: Yosry Ahmed <yosryahmed@google.com>
Nhat Pham Jan. 18, 2024, 6:16 p.m. UTC | #3 
On Wed, Jan 17, 2024 at 1:23 AM Chengming Zhou
<zhouchengming@bytedance.com> wrote:
>
> Not all zswap interfaces can handle the absence of the zswap rb-tree,
> actually only zswap_store() has handled it for now.
>
> To make things simple, we make sure each swapfile always have the
> zswap rb-tree prepared before being enabled and used. The preparation
> is unlikely to fail in practice, this patch just make it explicit.
>
> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>

This seems fine to me. IIUC, zswap_swapon() only fails when the rbtree
allocation fails, and the tree's memory footprint is small so that's
unlikely anyway.

Acked-by: Nhat Pham <nphamcs@gmail.com>



> ---
>  include/linux/zswap.h |  7 +++++--
>  mm/swapfile.c         | 10 +++++++---
>  mm/zswap.c            |  7 ++++---
>  3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/zswap.h b/include/linux/zswap.h
> index 0b709f5bc65f..eca388229d9a 100644
> --- a/include/linux/zswap.h
> +++ b/include/linux/zswap.h
> @@ -30,7 +30,7 @@ struct zswap_lruvec_state {
>  bool zswap_store(struct folio *folio);
>  bool zswap_load(struct folio *folio);
>  void zswap_invalidate(int type, pgoff_t offset);
> -void zswap_swapon(int type);
> +int zswap_swapon(int type);
>  void zswap_swapoff(int type);
>  void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg);
>  void zswap_lruvec_state_init(struct lruvec *lruvec);
> @@ -51,7 +51,10 @@ static inline bool zswap_load(struct folio *folio)
>  }
>
>  static inline void zswap_invalidate(int type, pgoff_t offset) {}
> -static inline void zswap_swapon(int type) {}
> +static inline int zswap_swapon(int type)
> +{
> +       return 0;
> +}
>  static inline void zswap_swapoff(int type) {}
>  static inline void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg) {}
>  static inline void zswap_lruvec_state_init(struct lruvec *lruvec) {}
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 3eec686484ef..6c53ea06626b 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -2347,8 +2347,6 @@ static void enable_swap_info(struct swap_info_struct *p, int prio,
>                                 unsigned char *swap_map,
>                                 struct swap_cluster_info *cluster_info)
>  {
> -       zswap_swapon(p->type);
> -
>         spin_lock(&swap_lock);
>         spin_lock(&p->lock);
>         setup_swap_info(p, prio, swap_map, cluster_info);
> @@ -3166,6 +3164,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>         if (error)
>                 goto bad_swap_unlock_inode;
>
> +       error = zswap_swapon(p->type);
> +       if (error)
> +               goto free_swap_address_space;
> +
>         /*
>          * Flush any pending IO and dirty mappings before we start using this
>          * swap device.
> @@ -3174,7 +3176,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>         error = inode_drain_writes(inode);
>         if (error) {
>                 inode->i_flags &= ~S_SWAPFILE;
> -               goto free_swap_address_space;
> +               goto free_swap_zswap;
>         }
>
>         mutex_lock(&swapon_mutex);
> @@ -3198,6 +3200,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
>
>         error = 0;
>         goto out;
> +free_swap_zswap:
> +       zswap_swapoff(p->type);
>  free_swap_address_space:
>         exit_swap_address_space(p->type);
>  bad_swap_unlock_inode:
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ca25b676048e..d88faea85978 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1519,7 +1519,7 @@ bool zswap_store(struct folio *folio)
>         if (folio_test_large(folio))
>                 return false;
>
> -       if (!zswap_enabled || !tree)
> +       if (!zswap_enabled)
>                 return false;
>
>         /*
> @@ -1772,19 +1772,20 @@ void zswap_invalidate(int type, pgoff_t offset)
>         spin_unlock(&tree->lock);
>  }
>
> -void zswap_swapon(int type)
> +int zswap_swapon(int type)
>  {
>         struct zswap_tree *tree;
>
>         tree = kzalloc(sizeof(*tree), GFP_KERNEL);
>         if (!tree) {
>                 pr_err("alloc failed, zswap disabled for swap type %d\n", type);
> -               return;
> +               return -ENOMEM;
>         }
>
>         tree->rbroot = RB_ROOT;
>         spin_lock_init(&tree->lock);
>         zswap_trees[type] = tree;
> +       return 0;
>  }
>
>  void zswap_swapoff(int type)
>
> --
> b4 0.10.1
diff mbox series

Patch

diff --git a/include/linux/zswap.h b/include/linux/zswap.h
index 0b709f5bc65f..eca388229d9a 100644
--- a/include/linux/zswap.h
+++ b/include/linux/zswap.h
@@ -30,7 +30,7 @@  struct zswap_lruvec_state {
 bool zswap_store(struct folio *folio);
 bool zswap_load(struct folio *folio);
 void zswap_invalidate(int type, pgoff_t offset);
-void zswap_swapon(int type);
+int zswap_swapon(int type);
 void zswap_swapoff(int type);
 void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg);
 void zswap_lruvec_state_init(struct lruvec *lruvec);
@@ -51,7 +51,10 @@  static inline bool zswap_load(struct folio *folio)
 }
 
 static inline void zswap_invalidate(int type, pgoff_t offset) {}
-static inline void zswap_swapon(int type) {}
+static inline int zswap_swapon(int type)
+{
+	return 0;
+}
 static inline void zswap_swapoff(int type) {}
 static inline void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg) {}
 static inline void zswap_lruvec_state_init(struct lruvec *lruvec) {}
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 3eec686484ef..6c53ea06626b 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -2347,8 +2347,6 @@  static void enable_swap_info(struct swap_info_struct *p, int prio,
 				unsigned char *swap_map,
 				struct swap_cluster_info *cluster_info)
 {
-	zswap_swapon(p->type);
-
 	spin_lock(&swap_lock);
 	spin_lock(&p->lock);
 	setup_swap_info(p, prio, swap_map, cluster_info);
@@ -3166,6 +3164,10 @@  SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	if (error)
 		goto bad_swap_unlock_inode;
 
+	error = zswap_swapon(p->type);
+	if (error)
+		goto free_swap_address_space;
+
 	/*
 	 * Flush any pending IO and dirty mappings before we start using this
 	 * swap device.
@@ -3174,7 +3176,7 @@  SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 	error = inode_drain_writes(inode);
 	if (error) {
 		inode->i_flags &= ~S_SWAPFILE;
-		goto free_swap_address_space;
+		goto free_swap_zswap;
 	}
 
 	mutex_lock(&swapon_mutex);
@@ -3198,6 +3200,8 @@  SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
 
 	error = 0;
 	goto out;
+free_swap_zswap:
+	zswap_swapoff(p->type);
 free_swap_address_space:
 	exit_swap_address_space(p->type);
 bad_swap_unlock_inode:
diff --git a/mm/zswap.c b/mm/zswap.c
index ca25b676048e..d88faea85978 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1519,7 +1519,7 @@  bool zswap_store(struct folio *folio)
 	if (folio_test_large(folio))
 		return false;
 
-	if (!zswap_enabled || !tree)
+	if (!zswap_enabled)
 		return false;
 
 	/*
@@ -1772,19 +1772,20 @@  void zswap_invalidate(int type, pgoff_t offset)
 	spin_unlock(&tree->lock);
 }
 
-void zswap_swapon(int type)
+int zswap_swapon(int type)
 {
 	struct zswap_tree *tree;
 
 	tree = kzalloc(sizeof(*tree), GFP_KERNEL);
 	if (!tree) {
 		pr_err("alloc failed, zswap disabled for swap type %d\n", type);
-		return;
+		return -ENOMEM;
 	}
 
 	tree->rbroot = RB_ROOT;
 	spin_lock_init(&tree->lock);
 	zswap_trees[type] = tree;
+	return 0;
 }
 
 void zswap_swapoff(int type)