On Wed, Jan 17, 2024 at 1:23 AM Chengming Zhou <zhouchengming@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxx> > --- > 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