On Tue, Apr 19, 2022 at 09:10:52PM -0700, Chaitanya Kulkarni wrote:
> We can avoid a function call virtblk_map_data() in the fast path if
> block layer request has no physical segments by moving the call
> blk_rq_nr_phys_segments() from virtblk_map_data() to virtio_queue_rq().
>
> Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
> ---
> drivers/block/virtio_blk.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
virtblk_map_data() is a static function that is not called by anything
else in virtio_blk.c. The disassembly on my x86_64 machine shows it has
been inlined into virtio_queue_rq(). The compiler has already done what
this patch is trying to do (and more).
Can you share performance data or some more background on why this
code change is necessary?
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index d038800474c2..74c3a48cd1e5 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -178,9 +178,6 @@ static int virtblk_map_data(struct blk_mq_hw_ctx *hctx, struct request *req,
> {
> int err;
>
> - if (!blk_rq_nr_phys_segments(req))
> - return 0;
> -
> vbr->sg_table.sgl = vbr->sg;
> err = sg_alloc_table_chained(&vbr->sg_table,
> blk_rq_nr_phys_segments(req),
> @@ -303,7 +300,7 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
> struct request *req = bd->rq;
> struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
> unsigned long flags;
> - int num;
> + int num = 0;
> int qid = hctx->queue_num;
> bool notify = false;
> blk_status_t status;
> @@ -315,10 +312,12 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
>
> blk_mq_start_request(req);
>
> - num = virtblk_map_data(hctx, req, vbr);
> - if (unlikely(num < 0)) {
> - virtblk_cleanup_cmd(req);
> - return BLK_STS_RESOURCE;
> + if (blk_rq_nr_phys_segments(req)) {
> + num = virtblk_map_data(hctx, req, vbr);
> + if (unlikely(num < 0)) {
> + virtblk_cleanup_cmd(req);
> + return BLK_STS_RESOURCE;
> + }
> }
>
> spin_lock_irqsave(&vblk->vqs[qid].lock, flags);
> --
> 2.29.0
>
Attachment:
signature.asc
Description: PGP signature
