Re: [PATCH 1/3] mm: zswap: interact directly with zsmalloc

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Sep 11, 2025 at 02:30:31PM +0000, Yosry Ahmed wrote:
> On Wed, Sep 10, 2025 at 09:42:40AM -0400, Johannes Weiner wrote:
> > @@ -314,6 +314,10 @@ static struct zswap_pool *__zswap_pool_create_fallback(void)
> >  		}
> >  	}
> >  
> > +	/* Kconfig bug? */
> > +	if (WARN_ON(!crypto_has_acomp(zswap_compressor, 0, 0)))
> > +		return NULL;
> > +
> >  	return zswap_pool_create(zswap_compressor);
> >  }
> 
> Sure, looks good, although I think it's clearer (and smaller diff) to
> preserve the old structure instead, up to you:
> 
> diff --git a/mm/zswap.c b/mm/zswap.c
> index c88ad61b232cf..bbfc087792648 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -300,18 +300,21 @@ static struct zswap_pool *zswap_pool_create(char *compressor)
> 
>  static struct zswap_pool *__zswap_pool_create_fallback(void)
>  {
> -       if (!crypto_has_acomp(zswap_compressor, 0, 0) &&
> +       bool has_comp = crypto_has_acomp(zswap_compressor, 0, 0);
> +
> +       if (!has_comp &&
>             strcmp(zswap_compressor, CONFIG_ZSWAP_COMPRESSOR_DEFAULT)) {
>                 pr_err("compressor %s not available, using default %s\n",
>                        zswap_compressor, CONFIG_ZSWAP_COMPRESSOR_DEFAULT);
>                 param_free_charp(&zswap_compressor);
>                 zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
> -               if (!crypto_has_acomp(zswap_compressor, 0, 0)) {
> -                       pr_err("default compressor %s not available\n",
> -                              zswap_compressor);
> -                       zswap_compressor = ZSWAP_PARAM_UNSET;
> -                       return NULL;
> -               }
> +               has_comp = crypto_has_acomp(zswap_compressor, 0, 0);
> +       }
> +       if (!has_comp) {
> +               pr_err("default compressor %s not available\n",
> +                      zswap_compressor);
> +               zswap_compressor = ZSWAP_PARAM_UNSET;
> +               return NULL;
>         }

No objection to moving the branch instead of adding another one. I'd
just like to retain the warning, since it shouldn't happen. And ditch
the bool, IMO it pointlessly splits the test from the consequences.

If you're fine with this Yosry, Andrew can you please fold it?

---

From b8fa4c7edd4f3c84853665b47acec8cebb4f4899 Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Mon, 15 Sep 2025 10:56:15 -0400
Subject: [PATCH] mm: zswap: interact directly with zsmalloc fix

Yosry points out that the default compressor check only applies when
something else is configured and we fall back, but not if it was
configured out of the box but isn't available. Move the test. Kconfig
should not permit this, so replace the pr_err() with a WARN.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/zswap.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index cba7077fda40..c1af782e54ec 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -309,12 +309,12 @@ static struct zswap_pool *__zswap_pool_create_fallback(void)
 		       zswap_compressor, CONFIG_ZSWAP_COMPRESSOR_DEFAULT);
 		param_free_charp(&zswap_compressor);
 		zswap_compressor = CONFIG_ZSWAP_COMPRESSOR_DEFAULT;
-		if (!crypto_has_acomp(zswap_compressor, 0, 0)) {
-			pr_err("default compressor %s not available\n",
-			       zswap_compressor);
-			zswap_compressor = ZSWAP_PARAM_UNSET;
-			return NULL;
-		}
+	}
+
+	/* Default compressor should be available. Kconfig bug? */
+	if (WARN_ON_ONCE(!crypto_has_acomp(zswap_compressor, 0, 0))) {
+		zswap_compressor = ZSWAP_PARAM_UNSET;
+		return NULL;
 	}
 
 	return zswap_pool_create(zswap_compressor);
-- 
2.51.0