Re: [PATCH] virtio-blk: support polling I/O

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Suwan Kim <suwan.kim027@gmail.com>
Cc: Max Gurtovoy <mgurtovoy@nvidia.com>,
	virtualization@lists.linux-foundation.org,
	linux-block@vger.kernel.org,
	Stefan Hajnoczi <stefanha@redhat.com>,
	pbonzini@redhat.com
Subject: Re: [PATCH] virtio-blk: support polling I/O
Date: Mon, 14 Mar 2022 11:12:50 -0400	[thread overview]
Message-ID: <20220314111239-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <Yi9BeVK3GbFrxIgB@localhost.localdomain>

On Mon, Mar 14, 2022 at 10:22:01PM +0900, Suwan Kim wrote:
> On Mon, Mar 14, 2022 at 07:15:18AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Mar 14, 2022 at 12:25:08PM +0200, Max Gurtovoy wrote:
> > > 
> > > On 3/14/2022 11:43 AM, Suwan Kim wrote:
> > > > On Sun, Mar 13, 2022 at 12:37:21PM +0200, Max Gurtovoy wrote:
> > > > > On 3/11/2022 6:07 PM, Suwan Kim wrote:
> > > > > > On Fri, Mar 11, 2022 at 10:38:07AM -0500, Michael S. Tsirkin wrote:
> > > > > > > On Sat, Mar 12, 2022 at 12:28:32AM +0900, Suwan Kim wrote:
> > > > > > > > diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> > > > > > > > index d888f013d9ff..3fcaf937afe1 100644
> > > > > > > > --- a/include/uapi/linux/virtio_blk.h
> > > > > > > > +++ b/include/uapi/linux/virtio_blk.h
> > > > > > > > @@ -119,8 +119,9 @@ struct virtio_blk_config {
> > > > > > > >    	 * deallocation of one or more of the sectors.
> > > > > > > >    	 */
> > > > > > > >    	__u8 write_zeroes_may_unmap;
> > > > > > > > +	__u8 unused1;
> > > > > > > > -	__u8 unused1[3];
> > > > > > > > +	__virtio16 num_poll_queues;
> > > > > > > >    } __attribute__((packed));
> > > > > > > Same as any virtio UAPI change, this has to go through the virtio TC.
> > > > > > > In particular I don't think gating a new config field on
> > > > > > > an existing feature flag is a good idea.
> > > > > > Did you mean that the polling should be based on a new feature like
> > > > > > "VIRTIO_BLK_F_POLL" and be added at the end of features_legacy[]
> > > > > > and features[]? If then, I will add the new feture flag and resend it.
> > > > > Isn't there a way in the SPEC today to create a queue without interrupt
> > > > > vector ?
> > > > It seems that it is not possible to create a queue without interrupt
> > > > vector. If it is possible, we can expect more polling improvement.
> > 
> > Yes, it's possible:
> > 
> > Writing a valid MSI-X Table entry number, 0 to 0x7FF, to
> > \field{config_msix_vector}/\field{queue_msix_vector} maps interrupts triggered
> > by the configuration change/selected queue events respectively to
> > the corresponding MSI-X vector. To disable interrupts for an
> > event type, the driver unmaps this event by writing a special NO_VECTOR
> > value:
> > 
> > \begin{lstlisting}
> > /* Vector value used to disable MSI for queue */
> > #define VIRTIO_MSI_NO_VECTOR            0xffff
> > \end{lstlisting}
>  
> Thanks for the information.
> 
> Then, in function vp_find_vqs_msix() at virtio_pci_common.c, it sets
> VIRTIO_MSI_NO_VECTOR if vritqueue->callback is NULL as below code.
> 
> static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> 		struct virtqueue *vqs[], vq_callback_t *callbacks[],
> ...
> 		if (!callbacks[i])
> 			msix_vec = VIRTIO_MSI_NO_VECTOR;
> ...
> 
> In oder to create poll queue in virtio-blk, I set NULL callback for
> poll virtqueues and it will create queue without irq.
> 
> Regards,
> Suwan Kim

Yes, it will.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization