Re: [PATCH v3 2/2] virtio-blk: support mq_ops->queue_rqs()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 24, 2022 at 11:04:50PM +0900, Suwan Kim wrote:
> @@ -367,6 +381,66 @@ static blk_status_t virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
>  	return BLK_STS_OK;
>  }
>  
> +static bool virtblk_prep_rq_batch(struct virtio_blk_vq *vq, struct request *req)
> +{
> +	struct virtio_blk *vblk = req->mq_hctx->queue->queuedata;
> +	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
> +	unsigned long flags;
> +	int num, err;
> +
> +	req->mq_hctx->tags->rqs[req->tag] = req;
> +
> +	if (virtblk_prep_rq(req->mq_hctx, vblk, req, vbr, &num) != BLK_STS_OK)
> +		return false;
> +
> +	spin_lock_irqsave(&vq->lock, flags);
> +	err = virtblk_add_req(vq->vq, vbr, vbr->sg_table.sgl, num);
> +	if (err) {
> +		spin_unlock_irqrestore(&vq->lock, flags);
> +		virtblk_unmap_data(req, vbr);
> +		virtblk_cleanup_cmd(req);
> +		return false;
> +	}
> +	spin_unlock_irqrestore(&vq->lock, flags);

Simplification:

  spin_lock_irqsave(&vq->lock, flags);
  err = virtblk_add_req(vq->vq, vbr, vbr->sg_table.sgl, num);
  spin_unlock_irqrestore(&vq->lock, flags);
  if (err) {
      virtblk_unmap_data(req, vbr);
      virtblk_cleanup_cmd(req);
      return false;
  }

> +
> +	return true;
> +}
> +
> +static void virtio_queue_rqs(struct request **rqlist)
> +{
> +	struct request *req, *next, *prev = NULL;
> +	struct request *requeue_list = NULL;
> +
> +	rq_list_for_each_safe(rqlist, req, next) {
> +		struct virtio_blk_vq *vq = req->mq_hctx->driver_data;
> +		unsigned long flags;
> +		bool kick;
> +
> +		if (!virtblk_prep_rq_batch(vq, req)) {
> +			rq_list_move(rqlist, &requeue_list, req, prev);
> +			req = prev;
> +
> +			if (!req)
> +				continue;
> +		}
> +
> +		if (!next || req->mq_hctx != next->mq_hctx) {
> +			spin_lock_irqsave(&vq->lock, flags);

Did you try calling virtblk_add_req() here to avoid acquiring and
releasing the lock multiple times? In other words, do virtblk_prep_rq()
but wait until we get here to do virtblk_add_req().

I don't know if it has any measurable effect on performance or maybe the
code would become too complex, but I noticed that we're not fully
exploiting batching.

> +			kick = virtqueue_kick_prepare(vq->vq);
> +			spin_unlock_irqrestore(&vq->lock, flags);
> +			if (kick)
> +				virtqueue_notify(vq->vq);
> +
> +			req->rq_next = NULL;
> +			*rqlist = next;
> +			prev = NULL;
> +		} else
> +			prev = req;

What guarantees that req is still alive after we called
virtblk_add_req()? The device may have seen it and completed it already
by the time we get here.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux