linux-kernel - Re: [PATCH] zram: drop pp_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADyq12yidNfeoRtUONM9VkdXpzZVbKy2+VYSJJpGqZtd7BqD2Q@mail.gmail.com>
Date: Tue, 16 Dec 2025 09:43:29 -0500
From: Brian Geffon <bgeffon@...gle.com>
To: Sergey Senozhatsky <senozhatsky@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Minchan Kim <minchan@...nel.org>, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, linux-block@...r.kernel.org, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] zram: drop pp_in_progress

On Tue, Dec 16, 2025 at 1:22 AM Sergey Senozhatsky
<senozhatsky@...omium.org> wrote:
>
> pp_in_progress makes sure that only one post-processing
> (writeback or recomrpession) is active at any given time.
> Functionality wise it, basically, shadows zram init_lock,
> when init_lock is acquired in writer mode.
>
> Switch recompress_store() and writeback_store() to take
> zram init_lock in writer mode, like all store() sysfs
> handlers should do, so that we can drop pp_in_progress.
> Recompression and writeback can be somewhat slow, so
> holding init_lock in writer mode can block zram attrs
> reads, but in reality the only zram attrs reads that
> take place are mm_stat reads, and usually it's the same
> process that reads mm_stat and does recompression or
> writeback.
>
> Suggested-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>

Reviewed-by: Brian Geffon <bgeffon@...gle.com>

> ---
>  drivers/block/zram/zram_drv.c | 28 ++++++----------------------
>  drivers/block/zram/zram_drv.h |  1 -
>  2 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 634848f45e9b..47826d8ed376 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1264,24 +1264,16 @@ static ssize_t writeback_store(struct device *dev,
>         ssize_t ret = len;
>         int err, mode = 0;
>
> -       guard(rwsem_read)(&zram->init_lock);
> +       guard(rwsem_write)(&zram->init_lock);
>         if (!init_done(zram))
>                 return -EINVAL;
>
> -       /* Do not permit concurrent post-processing actions. */
> -       if (atomic_xchg(&zram->pp_in_progress, 1))
> -               return -EAGAIN;
> -
> -       if (!zram->backing_dev) {
> -               ret = -ENODEV;
> -               goto out;
> -       }
> +       if (!zram->backing_dev)
> +               return -ENODEV;
>
>         pp_ctl = init_pp_ctl();
> -       if (!pp_ctl) {
> -               ret = -ENOMEM;
> -               goto out;
> -       }
> +       if (!pp_ctl)
> +               return -ENOMEM;
>
>         wb_ctl = init_wb_ctl(zram);
>         if (!wb_ctl) {
> @@ -1358,7 +1350,6 @@ static ssize_t writeback_store(struct device *dev,
>  out:
>         release_pp_ctl(zram, pp_ctl);
>         release_wb_ctl(wb_ctl);
> -       atomic_set(&zram->pp_in_progress, 0);
>
>         return ret;
>  }
> @@ -2622,14 +2613,10 @@ static ssize_t recompress_store(struct device *dev,
>         if (threshold >= huge_class_size)
>                 return -EINVAL;
>
> -       guard(rwsem_read)(&zram->init_lock);
> +       guard(rwsem_write)(&zram->init_lock);
>         if (!init_done(zram))
>                 return -EINVAL;
>
> -       /* Do not permit concurrent post-processing actions. */
> -       if (atomic_xchg(&zram->pp_in_progress, 1))
> -               return -EAGAIN;
> -
>         if (algo) {
>                 bool found = false;
>
> @@ -2700,7 +2687,6 @@ static ssize_t recompress_store(struct device *dev,
>         if (page)
>                 __free_page(page);
>         release_pp_ctl(zram, ctl);
> -       atomic_set(&zram->pp_in_progress, 0);
>         return ret;
>  }
>  #endif
> @@ -2891,7 +2877,6 @@ static void zram_reset_device(struct zram *zram)
>         zram->disksize = 0;
>         zram_destroy_comps(zram);
>         memset(&zram->stats, 0, sizeof(zram->stats));
> -       atomic_set(&zram->pp_in_progress, 0);
>         reset_bdev(zram);
>
>         comp_algorithm_set(zram, ZRAM_PRIMARY_COMP, default_compressor);
> @@ -3127,7 +3112,6 @@ static int zram_add(void)
>         zram->disk->fops = &zram_devops;
>         zram->disk->private_data = zram;
>         snprintf(zram->disk->disk_name, 16, "zram%d", device_id);
> -       atomic_set(&zram->pp_in_progress, 0);
>         zram_comp_params_reset(zram);
>         comp_algorithm_set(zram, ZRAM_PRIMARY_COMP, default_compressor);
>
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index 48d6861c6647..469a3dab44ad 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -143,6 +143,5 @@ struct zram {
>  #ifdef CONFIG_ZRAM_MEMORY_TRACKING
>         struct dentry *debugfs_dir;
>  #endif
> -       atomic_t pp_in_progress;
>  };
>  #endif
> --
> 2.52.0.239.gd5f0c6e74e-goog
>