From: Brian Geffon <bgeffon@google.com>
To: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Minchan Kim <minchan@kernel.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linux-block@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] zram: drop pp_in_progress
Date: Tue, 16 Dec 2025 09:43:29 -0500 [thread overview]
Message-ID: <CADyq12yidNfeoRtUONM9VkdXpzZVbKy2+VYSJJpGqZtd7BqD2Q@mail.gmail.com> (raw)
In-Reply-To: <20251216062223.647520-1-senozhatsky@chromium.org>
On Tue, Dec 16, 2025 at 1:22 AM Sergey Senozhatsky
<senozhatsky@chromium.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@linuxfoundation.org>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Brian Geffon <bgeffon@google.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
>
prev parent reply other threads:[~2025-12-16 14:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-16 6:22 Sergey Senozhatsky
2025-12-16 7:02 ` Sergey Senozhatsky
2025-12-16 14:43 ` Brian Geffon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADyq12yidNfeoRtUONM9VkdXpzZVbKy2+VYSJJpGqZtd7BqD2Q@mail.gmail.com \
--to=bgeffon@google.com \
--cc=akpm@linux-foundation.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=senozhatsky@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox