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 <[email protected]> Reviewed-by: Ming Lei <[email protected]> --- 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
