block/loop: improve performance

From: Shaohua Li <[email protected]>

Loop can handle any size of request. Limiting it to 255 sectors just burns the CPU for bio split and request merge for underlayer disk and also cause bad fs block allocation in directio mode.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/block/loop.c | 1 +
 1 file changed, 1 insertion(+)

drivers/block/loop.c +1 -0

index b55a1f8..428da07 100644
@@ -1799,6 +1799,7 @@ static int loop_add(struct loop_device **l, int i)
 	}
 	lo->lo_queue->queuedata = lo;
 
+	blk_queue_max_hw_sectors(lo->lo_queue, BLK_DEF_MAX_SECTORS);
 	/*
 	 * It doesn't make sense to enable merge because the I/O
 	 * submitted to backing file is handled page by page.

Signature · 1 line

2.9.5

From: Shaohua Li <[email protected]>

Currently loop disables merge. While it makes sense for buffer IO mode, directio mode can benefit from request merge. Without merge, loop could send small size IO to underlayer disk and harm performance.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/block/loop.c | 43 +++++++++++++++++++++++++++++++++++--------
 drivers/block/loop.h |  1 +
 2 files changed, 36 insertions(+), 8 deletions(-)

drivers/block/loop.c +0 -0

index 428da07..1bfa3ff 100644
@@ -213,10 +213,13 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
 	 */
 	blk_mq_freeze_queue(lo->lo_queue);

drivers/block/loop.c +28 -3

 	blk_mq_unfreeze_queue(lo->lo_queue);
 }
 

@@ -464,6 +467,8 @@ static void lo_rw_aio_complete(struct kiocb *iocb, long ret, long ret2)
 {
 	struct loop_cmd *cmd = container_of(iocb, struct loop_cmd, iocb);
 
+	kfree(cmd->bvec);
+	cmd->bvec = NULL;
 	cmd->ret = ret;
 	blk_mq_complete_request(cmd->rq);
 }

@@ -473,22 +478,44 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
 {
 	struct iov_iter iter;
 	struct bio_vec *bvec;
-	struct bio *bio = cmd->rq->bio;
+	struct request *rq = cmd->rq;
+	struct bio *bio = rq->bio;
 	struct file *file = lo->lo_backing_file;
+	unsigned int offset;
+	int segments = 0;
 	int ret;
 
-	/* nomerge for loop request queue */
-	WARN_ON(cmd->rq->bio != cmd->rq->biotail);
+	if (rq->bio != rq->biotail) {
+		struct req_iterator iter;
+		struct bio_vec tmp;
+
+		__rq_for_each_bio(bio, rq)
+			segments += bio_segments(bio);
+		bvec = kmalloc(sizeof(struct bio_vec) * segments, GFP_KERNEL);
+		if (!bvec)
+			return -EIO;
+		cmd->bvec = bvec;
+
+		rq_for_each_segment(tmp, rq, iter) {
+			*bvec = tmp;
+			bvec++;
+		}
+		bvec = cmd->bvec;
+		offset = 0;
+	} else {
+		offset = bio->bi_iter.bi_bvec_done;
+		bvec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
+		segments = bio_segments(bio);
+	}

drivers/block/loop.c +1 -1

-	iter.iov_offset = bio->bi_iter.bi_bvec_done;
+	iter.iov_offset = offset;
 
 	cmd->iocb.ki_pos = pos;
 	cmd->iocb.ki_filp = file;

drivers/block/loop.h +1 -0

index fecd3f9..bc9cf91 100644
@@ -72,6 +72,7 @@ struct loop_cmd {
 	bool use_aio;           /* use AIO interface to handle I/O */
 	long ret;
 	struct kiocb iocb;
+	struct bio_vec *bvec;
 };
 
 /* Support for loadable transfer modules */

Signature · 1 line

2.9.5

Quoted 5 lines · Shaohua Li

From: Shaohua Li <[email protected]>

Currently loop disables merge. While it makes sense for buffer IO mode, directio mode can benefit from request merge. Without merge, loop could send small size IO to underlayer disk and harm performance.

A couple of nits on comments below, besides that

Reviewed-by Omar Sandoval <[email protected]>

Quoted 13 lines

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/block/loop.c | 43 +++++++++++++++++++++++++++++++++++--------
 drivers/block/loop.h |  1 +
 2 files changed, 36 insertions(+), 8 deletions(-)

drivers/block/loop.c +0 -0

index 428da07..1bfa3ff 100644
@@ -213,10 +213,13 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
 	 */
 	blk_mq_freeze_queue(lo->lo_queue);

drivers/block/loop.c static void __loop_update_dio(struct loop_device *lo, bool dio) · line 222

Quoted 10 lines

drivers/block/loop.c +5 -2

 	lo->use_dio = use_dio;
-	if (use_dio)
+	if (use_dio) {
+		queue_flag_clear_unlocked(QUEUE_FLAG_NOMERGES, lo->lo_queue);
 		lo->lo_flags |= LO_FLAGS_DIRECT_IO;
-	else
+	} else {
+		queue_flag_set_unlocked(QUEUE_FLAG_NOMERGES, lo->lo_queue);
 		lo->lo_flags &= ~LO_FLAGS_DIRECT_IO;
+	}

Could you please also update the comment in loop_add() where we set QUEUE_FLAG_NOMERGES to say something like

/*
* By default we do buffered I/O, so it doesn't make sense to
* enable merging because the I/O submitted to backing file is
* handled page by page. We enable merging when switching to
* direct I/O mode.
*/

Quoted 48 lines

drivers/block/loop.c +28 -3

 	blk_mq_unfreeze_queue(lo->lo_queue);
 }
 

@@ -464,6 +467,8 @@ static void lo_rw_aio_complete(struct kiocb *iocb, long ret, long ret2)
 {
 	struct loop_cmd *cmd = container_of(iocb, struct loop_cmd, iocb);
 
+	kfree(cmd->bvec);
+	cmd->bvec = NULL;
 	cmd->ret = ret;
 	blk_mq_complete_request(cmd->rq);
 }

@@ -473,22 +478,44 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
 {
 	struct iov_iter iter;
 	struct bio_vec *bvec;
-	struct bio *bio = cmd->rq->bio;
+	struct request *rq = cmd->rq;
+	struct bio *bio = rq->bio;
 	struct file *file = lo->lo_backing_file;
+	unsigned int offset;
+	int segments = 0;
 	int ret;
 
-	/* nomerge for loop request queue */
-	WARN_ON(cmd->rq->bio != cmd->rq->biotail);
+	if (rq->bio != rq->biotail) {
+		struct req_iterator iter;
+		struct bio_vec tmp;
+
+		__rq_for_each_bio(bio, rq)
+			segments += bio_segments(bio);
+		bvec = kmalloc(sizeof(struct bio_vec) * segments, GFP_KERNEL);
+		if (!bvec)
+			return -EIO;
+		cmd->bvec = bvec;
+
+		rq_for_each_segment(tmp, rq, iter) {
+			*bvec = tmp;
+			bvec++;
+		}
+		bvec = cmd->bvec;
+		offset = 0;
+	} else {
+		offset = bio->bi_iter.bi_bvec_done;
+		bvec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
+		segments = bio_segments(bio);
+	}

drivers/block/loop.c static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd, · line 517

Quoted 10 lines

drivers/block/loop.c +1 -2

 
-	bvec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
 	iov_iter_bvec(&iter, ITER_BVEC | rw, bvec,
-		      bio_segments(bio), blk_rq_bytes(cmd->rq));
+		      segments, blk_rq_bytes(rq));
 	/*
 	 * This bio may be started from the middle of the 'bvec'
 	 * because of bio splitting, so offset from the bvec must
 	 * be passed to iov iterator
 	 */

This comment should be moved above to where you do

		offset = bio->bi_iter.bi_bvec_done;