Re: [PATCH v2 1/2] virtio-blk: set req->state to MQ_RQ_COMPLETE after polling I/O is finished

Re: [PATCH v2 1/2] virtio-blk: set req->state to MQ_RQ_COMPLETE after polling I/O is finished

[Christoph Hellwig]

On Wed, Dec 21, 2022 at 12:36:12AM +0900, Suwan Kim wrote:
> Driver should set req->state to MQ_RQ_COMPLETE after it finishes to process
> req. But virtio-blk doesn't set MQ_RQ_COMPLETE after virtblk_poll() handles
> req and req->state still remains MQ_RQ_IN_FLIGHT. Fortunately so far there
> is no issue about it because blk_mq_end_request_batch() sets req->state to
> MQ_RQ_IDLE.
> 
> In this patch, virblk_poll() calls blk_mq_complete_request_remote() to set
> req->state to MQ_RQ_COMPLETE before it adds req to a batch completion list.
> So it properly sets req->state after polling I/O is finished.
> 
> Fixes: 4e0400525691 ("virtio-blk: support polling I/O")
> Signed-off-by: Suwan Kim <suwan.kim027@gmail.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] virtio-blk: support completion batching for the IRQ path

[Christoph Hellwig]

> +		if (likely(!blk_should_fake_timeout(req->q)) &&
> +			!blk_mq_complete_request_remote(req) &&
> +			!blk_mq_add_to_batch(req, iob, vbr->status,
> +						virtblk_complete_batch))

One tab indents for line continuations are really confusing.  Please
make this:

		if (likely(!blk_should_fake_timeout(req->q)) &&
		    !blk_mq_complete_request_remote(req) &&
		    !blk_mq_add_to_batch(req, iob, vbr->status,
					 virtblk_complete_batch))

> +	found = virtblk_handle_req(vq, iob);
>  
>  	if (found)

You can drop the found variable here now:

	if (virtblk_handle_req(vq, iob))
 		blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization