* [PATCH] zram: drop pp_in_progress
@ 2025-12-16 6:22 Sergey Senozhatsky
2025-12-16 7:02 ` Sergey Senozhatsky
2025-12-16 14:43 ` Brian Geffon
0 siblings, 2 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2025-12-16 6:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Minchan Kim, Brian Geffon, linux-kernel, linux-mm, linux-block,
Sergey Senozhatsky, Greg Kroah-Hartman
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>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] zram: drop pp_in_progress
2025-12-16 6:22 [PATCH] zram: drop pp_in_progress Sergey Senozhatsky
@ 2025-12-16 7:02 ` Sergey Senozhatsky
2025-12-16 14:43 ` Brian Geffon
1 sibling, 0 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2025-12-16 7:02 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Minchan Kim, Brian Geffon, linux-kernel, linux-mm,
linux-block, Greg Kroah-Hartman
On (25/12/16 15:22), Sergey Senozhatsky 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>
Please disregard this one, there will be a follow-up patch.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] zram: drop pp_in_progress
2025-12-16 6:22 [PATCH] zram: drop pp_in_progress Sergey Senozhatsky
2025-12-16 7:02 ` Sergey Senozhatsky
@ 2025-12-16 14:43 ` Brian Geffon
1 sibling, 0 replies; 3+ messages in thread
From: Brian Geffon @ 2025-12-16 14:43 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Minchan Kim, linux-kernel, linux-mm, linux-block,
Greg Kroah-Hartman
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-16 14:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-16 6:22 [PATCH] zram: drop pp_in_progress Sergey Senozhatsky
2025-12-16 7:02 ` Sergey Senozhatsky
2025-12-16 14:43 ` Brian Geffon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox