Re: [PATCH v2] mm: z3fold: deprecate CONFIG_Z3FOLD

[Vitaly Wool]

On Thu, Sep 5, 2024 at 1:33 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
>
> The z3fold compressed pages allocator is rarely used, most users use
> zsmalloc. The only disadvantage of zsmalloc in comparison is the
> dependency on MMU, and zbud is a more common option for !MMU as it was
> the default zswap allocator for a long time.
>
> Historically, zsmalloc had worse latency than zbud and z3fold but
> offered better memory savings. This is no longer the case as shown by
> a simple recent analysis [1]. That analysis showed that z3fold does not
> have any advantage over zsmalloc or zbud considering both performance
> and memory usage. In a kernel build test on tmpfs in a limited cgroup,
> z3fold took 3% more time and used 1.8% more memory. The latency of
> zswap_load() was 7% higher, and that of zswap_store() was 10% higher.
> Zsmalloc is better in all metrics.
>
> Moreover, z3fold apparently has latent bugs, which was made noticeable
> by a recent soft lockup bug report with z3fold [2]. Switching to
> zsmalloc not only fixed the problem, but also reduced the swap usage
> from 6~8G to 1~2G. Other users have also reported being bitten by
> mistakenly enabling z3fold.
>
> Other than hurting users, z3fold is repeatedly causing wasted
> engineering effort. Apart from investigating the above bug, it came up
> in multiple development discussions (e.g. [3]) as something we need to
> handle, when there aren't any legit users (at least not intentionally).
>
> The natural course of action is to deprecate z3fold, and remove in a few
> cycles if no objections are raised from active users. Next on the list
> should be zbud, as it offers marginal latency gains at the cost of huge
> memory waste when compared to zsmalloc. That one will need to wait until
> zsmalloc does not depend on MMU.
>
> Rename the user-visible config option from CONFIG_Z3FOLD to
> CONFIG_Z3FOLD_DEPRECATED so that users with CONFIG_Z3FOLD=y get a new
> prompt with explanation during make oldconfig. Also, remove
> CONFIG_Z3FOLD=y from defconfigs.
>
> [1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndqF+3+_Vg@xxxxxxxxxxxxxx/
> [2]https://lore.kernel.org/lkml/EF0ABD3E-A239-4111-A8AB-5C442E759CF3@xxxxxxxxx/
> [3]https://lore.kernel.org/lkml/CAJD7tkbnmeVugfunffSovJf9FAgy9rhBVt_tx=nxUveLUfqVsA@xxxxxxxxxxxxxx/
>
> Acked-by: Chris Down <chris@xxxxxxxxxxxxxx>
> Acked-by: Nhat Pham <nphamcs@xxxxxxxxx>
> Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>

Acked-by: Vitaly Wool <vitaly.wool@xxxxxxxxxxxx>

> ---
>
> I think it should actually be fine to remove z3fold without deprecating
> it first, but I am doing the due diligence.
>
> v1: https://lore.kernel.org/linux-mm/20240112193103.3798287-1-yosryahmed@xxxxxxxxxx/
> v1 -> v2:
> - Make CONFIG_Z3FOLD_DEPRECATED a tristate option to match
>   CONFIG_Z3FOLD.
> - Update commit subject and log.
>
> ---
>  arch/loongarch/configs/loongson3_defconfig |  1 -
>  arch/powerpc/configs/ppc64_defconfig       |  1 -
>  mm/Kconfig                                 | 14 ++++++++++++--
>  3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
> index b4252c357c8e2..75b366407a60a 100644
> --- a/arch/loongarch/configs/loongson3_defconfig
> +++ b/arch/loongarch/configs/loongson3_defconfig
> @@ -96,7 +96,6 @@ CONFIG_ZPOOL=y
>  CONFIG_ZSWAP=y
>  CONFIG_ZSWAP_COMPRESSOR_DEFAULT_ZSTD=y
>  CONFIG_ZBUD=y
> -CONFIG_Z3FOLD=y
>  CONFIG_ZSMALLOC=m
>  # CONFIG_COMPAT_BRK is not set
>  CONFIG_MEMORY_HOTPLUG=y
> diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
> index 544a65fda77bc..d39284489aa26 100644
> --- a/arch/powerpc/configs/ppc64_defconfig
> +++ b/arch/powerpc/configs/ppc64_defconfig
> @@ -81,7 +81,6 @@ CONFIG_MODULE_SIG_SHA512=y
>  CONFIG_PARTITION_ADVANCED=y
>  CONFIG_BINFMT_MISC=m
>  CONFIG_ZSWAP=y
> -CONFIG_Z3FOLD=y
>  CONFIG_ZSMALLOC=y
>  # CONFIG_SLAB_MERGE_DEFAULT is not set
>  CONFIG_SLAB_FREELIST_RANDOM=y
> diff --git a/mm/Kconfig b/mm/Kconfig
> index b23913d4e47e2..536679d726417 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -177,15 +177,25 @@ config ZBUD
>           deterministic reclaim properties that make it preferable to a higher
>           density approach when reclaim will be used.
>
> -config Z3FOLD
> -       tristate "3:1 compression allocator (z3fold)"
> +config Z3FOLD_DEPRECATED
> +       tristate "3:1 compression allocator (z3fold) (DEPRECATED)"
>         depends on ZSWAP
>         help
> +         Deprecated and scheduled for removal in a few cycles. If you have
> +         a good reason for using Z3FOLD over ZSMALLOC, please contact
> +         linux-mm@xxxxxxxxx and the zswap maintainers.
> +
>           A special purpose allocator for storing compressed pages.
>           It is designed to store up to three compressed pages per physical
>           page. It is a ZBUD derivative so the simplicity and determinism are
>           still there.
>
> +config Z3FOLD
> +       tristate
> +       default y if Z3FOLD_DEPRECATED=y
> +       default m if Z3FOLD_DEPRECATED=m
> +       depends on Z3FOLD_DEPRECATED
> +
>  config ZSMALLOC
>         tristate
>         prompt "N:1 compression allocator (zsmalloc)" if ZSWAP
> --
> 2.46.0.469.g59c65b2a67-goog
>