[PATCH v2 8/8] ublk: lift checks out of ublk_{,un}map_io()

[Caleb Sander Mateos <csander@purestorage.com>]

ublk_map_io() and ublk_unmap_io() are no-ops for ublk devices that
enable user copy or zero copy, as well as for requests without data to
copy in the given direction. However, the implementation is a bit
convoluted, returning the full request data length and relying on the
caller to check the return value against the request length.
UBLK_F_SHMEM_ZC recently added branches to skip the ublk_{,un}map_io()
call for I/Os using a shared-memory buffer. This is a more logical place
for the device need_map and the ublk_need_{,un}map_req() checks, so move
them there from ublk_{,un}map_io().

Checking these conditions early also skips the expensive pointer-chasing
for the ublk_iod_is_shmem_zc() check in __ublk_complete_rq() for the
common case of a ublk device using user copy or zero copy.

Drop the req_op() filter in __ublk_complete_rq(), as it's redundant with
the ublk_need_unmap_req() check.

Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/block/ublk_drv.c | 70 +++++++++++-----------------------------
 1 file changed, 19 insertions(+), 51 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2f5de735b2d2..9249d25dceed 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1466,57 +1466,33 @@ static inline bool ublk_need_unmap_req(const struct request *req)
 {
 	return blk_rq_has_data(req) &&
 	       (req_op(req) == REQ_OP_READ || req_op(req) == REQ_OP_DRV_IN);
 }
 
-static unsigned int ublk_map_io(const struct ublk_queue *ubq,
-				const struct request *req,
+static unsigned int ublk_map_io(const struct request *req,
 				const struct ublk_io *io)
 {
-	const unsigned int rq_bytes = blk_rq_bytes(req);
+	struct iov_iter iter;
+	const int dir = ITER_DEST;
 
-	if (!ublk_need_map_io(ubq))
-		return rq_bytes;
-
-	/*
-	 * no zero copy, we delay copy WRITE request data into ublksrv
-	 * context and the big benefit is that pinning pages in current
-	 * context is pretty fast, see ublk_pin_user_pages
-	 */
-	if (ublk_need_map_req(req)) {
-		struct iov_iter iter;
-		const int dir = ITER_DEST;
-
-		if (import_ubuf(dir, u64_to_user_ptr(io->buf.addr), rq_bytes,
-				&iter) < 0)
-			return 0;
+	if (import_ubuf(dir, u64_to_user_ptr(io->buf.addr), blk_rq_bytes(req),
+			&iter) < 0)
+		return 0;
 
-		return ublk_copy_user_pages(req, 0, &iter, dir);
-	}
-	return rq_bytes;
+	return ublk_copy_user_pages(req, 0, &iter, dir);
 }
 
-static unsigned int ublk_unmap_io(bool need_map,
-		const struct request *req,
+static unsigned int ublk_unmap_io(const struct request *req,
 		const struct ublk_io *io)
 {
-	const unsigned int rq_bytes = blk_rq_bytes(req);
-
-	if (!need_map)
-		return rq_bytes;
-
-	if (ublk_need_unmap_req(req)) {
-		struct iov_iter iter;
-		const int dir = ITER_SOURCE;
+	struct iov_iter iter;
+	const int dir = ITER_SOURCE;
 
-		if (import_ubuf(dir, u64_to_user_ptr(io->buf.addr), io->res,
-				&iter) < 0)
-			return 0;
+	if (import_ubuf(dir, u64_to_user_ptr(io->buf.addr), io->res, &iter) < 0)
+		return 0;
 
-		return ublk_copy_user_pages(req, 0, &iter, dir);
-	}
-	return rq_bytes;
+	return ublk_copy_user_pages(req, 0, &iter, dir);
 }
 
 static bool ublk_validate_req(const struct ublk_queue *ubq,
 			      const struct request *req)
 {
@@ -1588,26 +1564,17 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
 	if (io->res < 0) {
 		res = errno_to_blk_status(io->res);
 		goto exit;
 	}
 
-	/*
-	 * FLUSH, DISCARD or WRITE_ZEROES usually won't return bytes returned, so end them
-	 * directly.
-	 *
-	 * Both the two needn't unmap.
-	 */
-	if (req_op(req) != REQ_OP_READ && req_op(req) != REQ_OP_WRITE &&
-	    req_op(req) != REQ_OP_DRV_IN)
-		goto exit;
-
 	/* shmem zero copy: no data to unmap, pages already shared */
-	if (ublk_iod_is_shmem_zc(req->mq_hctx->driver_data, req->tag))
+	if (!need_map || !ublk_need_unmap_req(req) ||
+	    ublk_iod_is_shmem_zc(req->mq_hctx->driver_data, req->tag))
 		goto exit;
 
 	/* for READ request, writing data in iod->addr to rq buffers */
-	unmapped_bytes = ublk_unmap_io(need_map, req, io);
+	unmapped_bytes = ublk_unmap_io(req, io);
 
 	/*
 	 * Extremely impossible since we got data filled in just before
 	 *
 	 * Re-read simply for this unlikely case.
@@ -1769,14 +1736,15 @@ static bool ublk_start_io(const struct ublk_queue *ubq, struct request *req,
 			  struct ublk_io *io)
 {
 	unsigned mapped_bytes;
 
 	/* shmem zero copy: skip data copy, pages already shared */
-	if (ublk_iod_is_shmem_zc(ubq, req->tag))
+	if (!ublk_need_map_io(ubq) || !ublk_need_map_req(req) ||
+	    ublk_iod_is_shmem_zc(ubq, req->tag))
 		return true;
 
-	mapped_bytes = ublk_map_io(ubq, req, io);
+	mapped_bytes = ublk_map_io(req, io);
 
 	/* partially mapped, update io descriptor */
 	if (unlikely(mapped_bytes != blk_rq_bytes(req))) {
 		/*
 		 * Nothing mapped, retry until we succeed.
-- 
2.54.0