diff mbox series

[v2] zram: remove double compression logic

Message ID 20220427100345.29461-1-avromanov@sberdevices.ru (mailing list archive)
State New, archived
Headers show
Series [v2] zram: remove double compression logic | expand

Commit Message

Alexey Romanov April 27, 2022, 10:03 a.m. UTC 
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.

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

Comments

Sergey Senozhatsky April 27, 2022, 11:30 a.m. UTC | #1 
On (22/04/27 13:03), Alexey Romanov wrote:
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index cb253d80d72b..4be6caf43b1d 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1153,9 +1153,8 @@ static ssize_t debug_stat_show(struct device *dev,
>  
>  	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);

I think this also has to bump `version` to 2, since format of the
file has changed.
Alexey Romanov April 28, 2022, 8:01 p.m. UTC | #2 
Thanks for the reply!

On Wed, Apr 27, 2022 at 08:30:16PM +0900, Sergey Senozhatsky wrote:
> On (22/04/27 13:03), Alexey Romanov wrote:
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index cb253d80d72b..4be6caf43b1d 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -1153,9 +1153,8 @@ static ssize_t debug_stat_show(struct device *dev,
> >  
> >  	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);
> 
> I think this also has to bump `version` to 2, since format of the
> file has changed.

Yes, I'll do that in the next patch.

Minchan, do you have any suggestions on this patch? 
I want to fix Sergey suggestion and sumbit next patch.
Minchan Kim May 2, 2022, 6:11 p.m. UTC | #3 
Hi Aleksey,

On Thu, Apr 28, 2022 at 08:01:12PM +0000, Aleksey Romanov wrote:
> Thanks for the reply!
> 
> On Wed, Apr 27, 2022 at 08:30:16PM +0900, Sergey Senozhatsky wrote:
> > On (22/04/27 13:03), Alexey Romanov wrote:
> > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > index cb253d80d72b..4be6caf43b1d 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -1153,9 +1153,8 @@ static ssize_t debug_stat_show(struct device *dev,
> > >  
> > >  	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);
> > 
> > I think this also has to bump `version` to 2, since format of the
> > file has changed.
> 
> Yes, I'll do that in the next patch.
> 
> Minchan, do you have any suggestions on this patch? 
> I want to fix Sergey suggestion and sumbit next patch.

You need to remove zs_free in the path "Compression failed"

Since we don't have double compression, we don't need QUEUE_FLAG_STABLE_WRITES
so you could remove QUEUE_FLAG_STABLE_WRITES, too.

Thanks.
diff mbox series

Patch

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cb253d80d72b..4be6caf43b1d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1153,9 +1153,8 @@  static ssize_t debug_stat_show(struct device *dev,
 
 	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);
@@ -1388,33 +1386,15 @@  static int __zram_bvec_write(struct zram *zram, struct bio_vec *bvec,
 
 	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;
 	}
 
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 */