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 <[email protected]>
> ---
>  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

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to