linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] zram: remove double compression logic
@ 2022-05-05  9:44 Alexey Romanov
  2022-05-05 16:57 ` Minchan Kim
  2022-08-09 12:41 ` Sergey Senozhatsky
  0 siblings, 2 replies; 4+ messages in thread
From: Alexey Romanov @ 2022-05-05  9:44 UTC (permalink / raw)
  To: akpm
  Cc: minchan, ngupta, senozhatsky, linux-block, axboe, kernel,
	linux-kernel, mnitenko, Alexey Romanov, Dmitry Rokosov

The 2nd trial allocation under per-cpu presumption has been used to
prevent regression of allocation failure. However, it makes trouble
for maintenance without significant benefit. The slowpath branch is
executed extremely rarely: getting there is problematic. Therefore,
we delete this branch.

Since b09ab054b69b, zram has used QUEUE_FLAG_STABLE_WRITES to prevent
buffer change between 1st and 2nd memory allocations. Since we remove
second trial memory allocation logic, we could remove the STABLE_WRITES
flag because there is no change buffer to be modified under us.

Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/block/zram/zram_drv.c | 42 +++++++++--------------------------
 drivers/block/zram/zram_drv.h |  1 -
 2 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cb253d80d72b..e10066a10dcf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1147,15 +1147,14 @@ static ssize_t bd_stat_show(struct device *dev,
 static ssize_t debug_stat_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
-	int version = 1;
+	int version = 2;
 	struct zram *zram = dev_to_zram(dev);
 	ssize_t ret;
 
 	down_read(&zram->init_lock);
 	ret = scnprintf(buf, PAGE_SIZE,
-			"version: %d\n%8llu %8llu\n",
+			"version: %d\n%8llu\n",
 			version,
-			(u64)atomic64_read(&zram->stats.writestall),
 			(u64)atomic64_read(&zram->stats.miss_free));
 	up_read(&zram->init_lock);
 
@@ -1373,7 +1372,6 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	}
 	kunmap_atomic(mem);
 
-compress_again:
 	zstrm = zcomp_stream_get(zram->comp);
 	src = kmap_atomic(page);
 	ret = zcomp_compress(zstrm, src, &comp_len);
@@ -1382,39 +1380,20 @@ static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 	if (unlikely(ret)) {
 		zcomp_stream_put(zram->comp);
 		pr_err("Compression failed! err=%d\n", ret);
-		zs_free(zram->mem_pool, handle);
 		return ret;
 	}
 
 	if (comp_len >= huge_class_size)
 		comp_len = PAGE_SIZE;
-	/*
-	 * handle allocation has 2 paths:
-	 * a) fast path is executed with preemption disabled (for
-	 *  per-cpu streams) and has __GFP_DIRECT_RECLAIM bit clear,
-	 *  since we can't sleep;
-	 * b) slow path enables preemption and attempts to allocate
-	 *  the page with __GFP_DIRECT_RECLAIM bit set. we have to
-	 *  put per-cpu compression stream and, thus, to re-do
-	 *  the compression once handle is allocated.
-	 *
-	 * if we have a 'non-null' handle here then we are coming
-	 * from the slow path and handle has already been allocated.
-	 */
-	if (!handle)
-		handle = zs_malloc(zram->mem_pool, comp_len,
-				__GFP_KSWAPD_RECLAIM |
-				__GFP_NOWARN |
-				__GFP_HIGHMEM |
-				__GFP_MOVABLE);
-	if (!handle) {
+
+	handle = zs_malloc(zram->mem_pool, comp_len,
+			__GFP_KSWAPD_RECLAIM |
+			__GFP_NOWARN |
+			__GFP_HIGHMEM |
+			__GFP_MOVABLE);
+
+	if (unlikely(!handle)) {
 		zcomp_stream_put(zram->comp);
-		atomic64_inc(&zram->stats.writestall);
-		handle = zs_malloc(zram->mem_pool, comp_len,
-				GFP_NOIO | __GFP_HIGHMEM |
-				__GFP_MOVABLE);
-		if (handle)
-			goto compress_again;
 		return -ENOMEM;
 	}
 
@@ -1975,7 +1954,6 @@ static int zram_add(void)
 	if (ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE)
 		blk_queue_max_write_zeroes_sectors(zram->disk->queue, UINT_MAX);
 
-	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue);
 	ret = device_add_disk(NULL, zram->disk, zram_disk_groups);
 	if (ret)
 		goto out_cleanup_disk;
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 80c3b43b4828..158c91e54850 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -81,7 +81,6 @@ struct zram_stats {
 	atomic64_t huge_pages_since;	/* no. of huge pages since zram set up */
 	atomic64_t pages_stored;	/* no. of pages currently stored */
 	atomic_long_t max_used_pages;	/* no. of maximum pages stored */
-	atomic64_t writestall;		/* no. of write slow paths */
 	atomic64_t miss_free;		/* no. of missed free */
 #ifdef	CONFIG_ZRAM_WRITEBACK
 	atomic64_t bd_count;		/* no. of pages in backing device */
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] zram: remove double compression logic
  2022-05-05  9:44 [PATCH v5] zram: remove double compression logic Alexey Romanov
@ 2022-05-05 16:57 ` Minchan Kim
  2022-08-09 12:41 ` Sergey Senozhatsky
  1 sibling, 0 replies; 4+ messages in thread
From: Minchan Kim @ 2022-05-05 16:57 UTC (permalink / raw)
  To: Alexey Romanov
  Cc: akpm, ngupta, senozhatsky, linux-block, axboe, kernel,
	linux-kernel, mnitenko, Dmitry Rokosov

On Thu, May 05, 2022 at 12:44:43PM +0300, Alexey Romanov wrote:
> The 2nd trial allocation under per-cpu presumption has been used to
> prevent regression of allocation failure. However, it makes trouble
> for maintenance without significant benefit. The slowpath branch is
> executed extremely rarely: getting there is problematic. Therefore,
> we delete this branch.
> 
> Since b09ab054b69b, zram has used QUEUE_FLAG_STABLE_WRITES to prevent
> buffer change between 1st and 2nd memory allocations. Since we remove
> second trial memory allocation logic, we could remove the STABLE_WRITES
> flag because there is no change buffer to be modified under us.
> 
> Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>

Acked-by: Minchan Kim <minchan@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] zram: remove double compression logic
  2022-05-05  9:44 [PATCH v5] zram: remove double compression logic Alexey Romanov
  2022-05-05 16:57 ` Minchan Kim
@ 2022-08-09 12:41 ` Sergey Senozhatsky
  2022-08-09 13:06   ` Aleksey Romanov
  1 sibling, 1 reply; 4+ messages in thread
From: Sergey Senozhatsky @ 2022-08-09 12:41 UTC (permalink / raw)
  To: Alexey Romanov, Minchan Kim
  Cc: akpm, ngupta, senozhatsky, linux-block, axboe, kernel,
	linux-kernel, mnitenko, Dmitry Rokosov

On (22/05/05 12:44), Alexey Romanov wrote:
> @@ -1975,7 +1954,6 @@ static int zram_add(void)
>  	if (ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE)
>  		blk_queue_max_write_zeroes_sectors(zram->disk->queue, UINT_MAX);
>  
> -	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue);

By the way, why did it remove QUEUE_FLAG_STABLE_WRITES bit?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v5] zram: remove double compression logic
  2022-08-09 12:41 ` Sergey Senozhatsky
@ 2022-08-09 13:06   ` Aleksey Romanov
  0 siblings, 0 replies; 4+ messages in thread
From: Aleksey Romanov @ 2022-08-09 13:06 UTC (permalink / raw)
  To: Sergey Senozhatsky, minchan
  Cc: akpm, ngupta, linux-block, axboe, kernel, linux-kernel, mnitenko,
	Dmitry Rokosov

Hi Sergey,

On Tue, Aug 09, 2022 at 09:41:22PM +0900, Sergey Senozhatsky wrote:
> On (22/05/05 12:44), Alexey Romanov wrote:
> > @@ -1975,7 +1954,6 @@ static int zram_add(void)
> >  	if (ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE)
> >  		blk_queue_max_write_zeroes_sectors(zram->disk->queue, UINT_MAX);
> >  
> > -	blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue);
> 
> By the way, why did it remove QUEUE_FLAG_STABLE_WRITES bit?

Minchan asked me to add this change and this description:

"Since b09ab054b69b, zram has used QUEUE_FLAG_STABLE_WRITES to prevent
buffer change between 1st and 2nd memory allocations. Since we remove
second trial memory allocation logic, we could remove the STABLE_WRITES
flag because there is no change buffer to be modified under us"

This seems to be logical.

-- 
Thank you,
Alexey

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-09 13:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05  9:44 [PATCH v5] zram: remove double compression logic Alexey Romanov
2022-05-05 16:57 ` Minchan Kim
2022-08-09 12:41 ` Sergey Senozhatsky
2022-08-09 13:06   ` Aleksey Romanov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).