* [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq
@ 2016-08-10 15:24 ggarcia
2016-08-13 0:31 ` David Miller
2016-08-14 23:16 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq Michael S. Tsirkin
0 siblings, 2 replies; 5+ messages in thread
From: ggarcia @ 2016-08-10 15:24 UTC (permalink / raw)
To: netdev; +Cc: stefanha, mst, Gerard Garcia
From: Gerard Garcia <ggarcia@deic.uab.cat>
Remove unnecessary use of enable/disable callback notifications
and the incorrect more space available check.
The virtio_transport_tx_work handles when the TX virtqueue
has more buffers available.
Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
* Comment style.
net/vmw_vsock/virtio_transport.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 699dfab..936d7ee 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -87,9 +87,6 @@ virtio_transport_send_pkt_work(struct work_struct *work)
vq = vsock->vqs[VSOCK_VQ_TX];
- /* Avoid unnecessary interrupts while we're processing the ring */
- virtqueue_disable_cb(vq);
-
for (;;) {
struct virtio_vsock_pkt *pkt;
struct scatterlist hdr, buf, *sgs[2];
@@ -99,7 +96,6 @@ virtio_transport_send_pkt_work(struct work_struct *work)
spin_lock_bh(&vsock->send_pkt_list_lock);
if (list_empty(&vsock->send_pkt_list)) {
spin_unlock_bh(&vsock->send_pkt_list_lock);
- virtqueue_enable_cb(vq);
break;
}
@@ -118,13 +114,13 @@ virtio_transport_send_pkt_work(struct work_struct *work)
}
ret = virtqueue_add_sgs(vq, sgs, out_sg, in_sg, pkt, GFP_KERNEL);
+ /* Usually this means that there is no more space available in
+ * the vq
+ */
if (ret < 0) {
spin_lock_bh(&vsock->send_pkt_list_lock);
list_add(&pkt->list, &vsock->send_pkt_list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
-
- if (!virtqueue_enable_cb(vq) && ret == -ENOSPC)
- continue; /* retry now that we have more space */
break;
}
--
2.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq
2016-08-10 15:24 [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq ggarcia
@ 2016-08-13 0:31 ` David Miller
2016-08-13 9:55 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TXvq Gerard Garcia
2016-08-14 23:16 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2016-08-13 0:31 UTC (permalink / raw)
To: ggarcia; +Cc: netdev, stefanha, mst
From: ggarcia@abra.uab.cat
Date: Wed, 10 Aug 2016 17:24:34 +0200
> From: Gerard Garcia <ggarcia@deic.uab.cat>
>
> Remove unnecessary use of enable/disable callback notifications
> and the incorrect more space available check.
>
> The virtio_transport_tx_work handles when the TX virtqueue
> has more buffers available.
>
> Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
This does not apply cleanly to the current net GIT tree.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] VSOCK: remove more space available check filling TXvq
2016-08-13 0:31 ` David Miller
@ 2016-08-13 9:55 ` Gerard Garcia
0 siblings, 0 replies; 5+ messages in thread
From: Gerard Garcia @ 2016-08-13 9:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, stefanha, mst
On 08/13/2016 02:31 AM, David Miller wrote:
> From: ggarcia@abra.uab.cat
> Date: Wed, 10 Aug 2016 17:24:34 +0200
>
>> From: Gerard Garcia <ggarcia@deic.uab.cat>
>>
>> Remove unnecessary use of enable/disable callback notifications
>> and the incorrect more space available check.
>>
>> The virtio_transport_tx_work handles when the TX virtqueue
>> has more buffers available.
>>
>> Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat>
>> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> This does not apply cleanly to the current net GIT tree.
>
I'm sorry, I should have said that it applies over the mst vhost tree.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq
2016-08-10 15:24 [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq ggarcia
2016-08-13 0:31 ` David Miller
@ 2016-08-14 23:16 ` Michael S. Tsirkin
2016-08-15 14:29 ` Stefan Hajnoczi
1 sibling, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2016-08-14 23:16 UTC (permalink / raw)
To: ggarcia; +Cc: netdev, stefanha
On Wed, Aug 10, 2016 at 05:24:34PM +0200, ggarcia@abra.uab.cat wrote:
> From: Gerard Garcia <ggarcia@deic.uab.cat>
>
> Remove unnecessary use of enable/disable callback notifications
> and the incorrect more space available check.
>
> The virtio_transport_tx_work handles when the TX virtqueue
> has more buffers available.
>
> Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat>
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Generally enable/disable is a worthwhile optimization,
but if Stefan wants to keep code simple for now,
I don't have a problem with that.
> ---
>
> v2:
> * Comment style.
>
> net/vmw_vsock/virtio_transport.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 699dfab..936d7ee 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -87,9 +87,6 @@ virtio_transport_send_pkt_work(struct work_struct *work)
>
> vq = vsock->vqs[VSOCK_VQ_TX];
>
> - /* Avoid unnecessary interrupts while we're processing the ring */
> - virtqueue_disable_cb(vq);
> -
> for (;;) {
> struct virtio_vsock_pkt *pkt;
> struct scatterlist hdr, buf, *sgs[2];
> @@ -99,7 +96,6 @@ virtio_transport_send_pkt_work(struct work_struct *work)
> spin_lock_bh(&vsock->send_pkt_list_lock);
> if (list_empty(&vsock->send_pkt_list)) {
> spin_unlock_bh(&vsock->send_pkt_list_lock);
> - virtqueue_enable_cb(vq);
> break;
> }
>
> @@ -118,13 +114,13 @@ virtio_transport_send_pkt_work(struct work_struct *work)
> }
>
> ret = virtqueue_add_sgs(vq, sgs, out_sg, in_sg, pkt, GFP_KERNEL);
> + /* Usually this means that there is no more space available in
> + * the vq
> + */
> if (ret < 0) {
> spin_lock_bh(&vsock->send_pkt_list_lock);
> list_add(&pkt->list, &vsock->send_pkt_list);
> spin_unlock_bh(&vsock->send_pkt_list_lock);
> -
> - if (!virtqueue_enable_cb(vq) && ret == -ENOSPC)
> - continue; /* retry now that we have more space */
> break;
> }
>
> --
> 2.9.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq
2016-08-14 23:16 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq Michael S. Tsirkin
@ 2016-08-15 14:29 ` Stefan Hajnoczi
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2016-08-15 14:29 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: ggarcia, netdev
[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]
On Mon, Aug 15, 2016 at 02:16:51AM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 10, 2016 at 05:24:34PM +0200, ggarcia@abra.uab.cat wrote:
> > From: Gerard Garcia <ggarcia@deic.uab.cat>
> >
> > Remove unnecessary use of enable/disable callback notifications
> > and the incorrect more space available check.
> >
> > The virtio_transport_tx_work handles when the TX virtqueue
> > has more buffers available.
> >
> > Signed-off-by: Gerard Garcia <ggarcia@deic.uab.cat>
> > Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Generally enable/disable is a worthwhile optimization,
> but if Stefan wants to keep code simple for now,
> I don't have a problem with that.
Enabling/disabling callbacks is inappropriate in this function because
its job is *submitting* buffers rather than *reclaiming* completed
buffers. Oops, I'm not sure how this mistake crept in :).
The callback suppression should only be done around the loop that
reclaims buffers. Doing it in the wrong place leads to hangs if
interrupts are lost.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-15 14:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-10 15:24 [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq ggarcia
2016-08-13 0:31 ` David Miller
2016-08-13 9:55 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TXvq Gerard Garcia
2016-08-14 23:16 ` [PATCH v2 1/1] VSOCK: remove more space available check filling TX vq Michael S. Tsirkin
2016-08-15 14:29 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.