[2/3] zram: consolidate zram_bio_read()_zram_bio_write()

Commit Message

Chaitanya Kulkarni May 12, 2023, 8:29 a.m. UTC

zram_bio_read() and zram_bio_write() are 26 lines rach and share most of
the code except call to zram_bdev_read() and zram_bvec_write() calls.
Consolidate code into single zram_bio_rw() to remove the duplicate code
and an extra function that is only needed for 2 lines of code :-

1c1
< static void zram_bio_read(struct zram *zram, struct bio *bio)
---
> static void zram_bio_write(struct zram *zram, struct bio *bio)
13,14c13,14
< 		if (zram_bvec_read(zram, &bv, index, offset, bio) < 0) {
< 			atomic64_inc(&zram->stats.failed_reads);
---
> 		if (zram_bvec_write(zram, &bv, index, offset, bio) < 0) {
> 			atomic64_inc(&zram->stats.failed_writes);
18d17
< 		flush_dcache_page(bv.bv_page);

Comments

Christoph Hellwig May 12, 2023, 2:32 p.m. UTC | #1

On Fri, May 12, 2023 at 01:29:57AM -0700, Chaitanya Kulkarni wrote:
> zram_bio_read() and zram_bio_write() are 26 lines rach and share most of
> the code except call to zram_bdev_read() and zram_bvec_write() calls.
> Consolidate code into single zram_bio_rw() to remove the duplicate code
> and an extra function that is only needed for 2 lines of code :-

And it adds an extra conditional to every page in the fast path..

Chaitanya Kulkarni May 13, 2023, 1:06 a.m. UTC | #2

On 5/12/23 07:32, Christoph Hellwig wrote:
> On Fri, May 12, 2023 at 01:29:57AM -0700, Chaitanya Kulkarni wrote:
>> zram_bio_read() and zram_bio_write() are 26 lines rach and share most of
>> the code except call to zram_bdev_read() and zram_bvec_write() calls.
>> Consolidate code into single zram_bio_rw() to remove the duplicate code
>> and an extra function that is only needed for 2 lines of code :-
> And it adds an extra conditional to every page in the fast path..

will create a version without additional branch in v2 ..

-ck

Patch

diff stats with this patch :-

 drivers/block/zram/zram_drv.c | 53 ++++++++++++-----------------------
 1 file changed, 18 insertions(+), 35 deletions(-)

Signed-off-by: Chaitanya Kulkarni <[e-mail]>
---
 drivers/block/zram/zram_drv.c | 53 ++++++++++++-----------------------
 1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b2e419f15f71..fc37419b3735 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1873,38 +1873,12 @@  static void zram_bio_discard(struct zram *zram, struct bio *bio)
 	bio_endio(bio);
 }
 
-static void zram_bio_read(struct zram *zram, struct bio *bio)
-{
-	struct bvec_iter iter;
-	struct bio_vec bv;
-	unsigned long start_time;
-
-	start_time = bio_start_io_acct(bio);
-	bio_for_each_segment(bv, bio, iter) {
-		u32 index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
-
-		if (zram_bvec_read(zram, &bv, index, offset, bio) < 0) {
-			atomic64_inc(&zram->stats.failed_reads);
-			bio->bi_status = BLK_STS_IOERR;
-			break;
-		}
-		flush_dcache_page(bv.bv_page);
-
-		zram_slot_lock(zram, index);
-		zram_accessed(zram, index);
-		zram_slot_unlock(zram, index);
-	}
-	bio_end_io_acct(bio, start_time);
-	bio_endio(bio);
-}
-
-static void zram_bio_write(struct zram *zram, struct bio *bio)
+static void zram_bio_rw(struct zram *zram, struct bio *bio)
 {
 	struct bvec_iter iter;
 	struct bio_vec bv;
 	unsigned long start_time;
+	int ret;
 
 	start_time = bio_start_io_acct(bio);
 	bio_for_each_segment(bv, bio, iter) {
@@ -1912,10 +1886,21 @@  static void zram_bio_write(struct zram *zram, struct bio *bio)
 		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
 				SECTOR_SHIFT;
 
-		if (zram_bvec_write(zram, &bv, index, offset, bio) < 0) {
-			atomic64_inc(&zram->stats.failed_writes);
-			bio->bi_status = BLK_STS_IOERR;
-			break;
+		if (op_is_write(bio_op(bio))) {
+			ret = zram_bvec_write(zram, &bv, index, offset, bio);
+			if (ret < 0) {
+				atomic64_inc(&zram->stats.failed_writes);
+				bio->bi_status = BLK_STS_IOERR;
+				break;
+			}
+		} else {
+			ret = zram_bvec_read(zram, &bv, index, offset, bio);
+			if (ret < 0) {
+				atomic64_inc(&zram->stats.failed_reads);
+				bio->bi_status = BLK_STS_IOERR;
+				break;
+			}
+			flush_dcache_page(bv.bv_page);
 		}
 
 		zram_slot_lock(zram, index);
@@ -1935,10 +1920,8 @@  static void zram_submit_bio(struct bio *bio)
 
 	switch (bio_op(bio)) {
 	case REQ_OP_READ:
-		zram_bio_read(zram, bio);
-		break;
 	case REQ_OP_WRITE:
-		zram_bio_write(zram, bio);
+		zram_bio_rw(zram, bio);
 		break;
 	case REQ_OP_DISCARD:
 	case REQ_OP_WRITE_ZEROES: