Re: [PATCH v2 1/1] mm/zswap: enable zswap_ever_enabled in zswap_pool_create()

[Yosry Ahmed]

On Sun, Sep 6, 2026 at 6:37 AM Longlong Xia <xialonglong2025@xxxxxxx> wrote:
>
> From: Longlong Xia <xialonglong@xxxxxxxxxx>
>
> If zswap is enabled by default at boot and pool creation fails, then a
> pool is later created by updating the compressor, data written to
> zswap is corrupted on swapin.
>
> Enable the static key in zswap_pool_create(), covering boot-time and
> runtime pool creation with a single site.
>
> Verified with fault injection on a stock kernel (compressor builtin,
> CONFIG_ZSWAP_DEFAULT_ON=n):
>
>   1. Boot with zswap.enabled=1; pool creation fails, init completes
>      pool-less (static key off).
>   2. Echo an available compressor name to zswap.compressor.
>   3. Enable zswap.
>   4. madvise(MADV_PAGEOUT) a pattern-verified 512 MiB region, then
>      fault it back in and verify.
>
> Step 4 reads back 131072/131072 zeroed pages (zswpin=0, zswpout=131072)
> without this patch; all pages intact (zswpin=131072) with it.
>
> Fixes: 2d4d2b1cfb85 ("mm: zswap: add zswap_never_enabled()")
> Suggested-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Zcode:GLM-5.3
> Signed-off-by: Longlong Xia <xialonglong@xxxxxxxxxx>
> ---
> Changes in v2:
> - Enable the static key in zswap_pool_create() instead of adding a
>   second site next to the compressor parameter update, as suggested
>   by Yosry; boot-time and runtime pool creation are now covered by a
>   single site, and the zswap_setup() site is gone.
> - Use the high-level description from Yosry's reply to Andrew for the
>   corruption scenario, and rename the subject accordingly.
>
> Link: https://lore.kernel.org/r/20260905125101.2970456-1-xialonglong2025@xxxxxxx
>
>  mm/zswap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 37f34e406c8e3..6dfb6ae709e97 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -324,6 +324,9 @@ static struct zswap_pool *zswap_pool_create(char *compressor)
>
>         zswap_pool_debug("created", pool);
>
> +       /* Enable the key here so every pool creation path is covered. */
> +       static_branch_enable(&zswap_ever_enabled);

I would drop the comment, otherwise LGTM:

Acked-by: Yosry Ahmed <yosry@xxxxxxxxxx>

> +
>         return pool;
>
>  ref_fail:
> @@ -1805,7 +1808,6 @@ static int zswap_setup(void)
>                 pr_info("loaded using pool %s\n", pool->tfm_name);
>                 list_add(&pool->list, &zswap_pools);
>                 zswap_has_pool = true;
> -               static_branch_enable(&zswap_ever_enabled);
>         } else {
>                 pr_err("pool creation failed\n");
>                 zswap_enabled = false;
> --
> 2.43.0
>