Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback

From: Jason Wang
Date: Mon May 21 2012 - 02:05:32 EST


On 05/18/2012 11:29 PM, Shirley Ma wrote:
On Fri, 2012-05-18 at 17:58 +0800, Jason Wang wrote:
On 05/17/2012 11:34 PM, Shirley Ma wrote:
On Thu, 2012-05-17 at 10:50 +0800, Jason Wang wrote:
The problem is we may stop the tx queue when there no enough
capacity
to
place packets, at this moment we depends on the tx interrupt to
re-enable the tx queue. So if we didn't poll the vhost during
callback,
guest may lose the tx interrupt to re-enable the tx queue which
could
stall the whole tx queue.
VHOST_MAX_PEND should handle the capacity.

Hasn't the above situation been handled in handle_tx() code?:
...
if (unlikely(num_pends> VHOST_MAX_PEND)) {
tx_poll_start(net, sock);

set_bit(SOCK_ASYNC_NOSPACE,&sock->flags);
break;
}
...

Thanks
Shirley
It may not help in because:

- tx polling depends on skb_orphan() which is often called by device
driver when it place the packet into the queue of the devices instead
of when the packets were sent. So it was too early for vhost to be
notified.
Then do you think it's better to replace with vhost_poll_queue here
instead?

Just like what does this patch do - calling vhost_poll_queue() in vhost_zerocopy_callback().
- it only works when the pending DMAs exceeds VHOST_MAX_PEND, it's
highly possible that guest needs to be notified when the pending
packets
isn't so much.
In which situation the guest needs to be notified when there is no TX
besides buffers run out?

Consider guest call virtqueue_enable_cb_delayed() which means it only need to be notified when 3/4 of pending buffers ( about 178 buffers (256-MAX_SKB_FRAGS-2)*3/4 ) were sent by host. So vhost_net would notify guest when about 60 buffers were pending. Since tx polling is only enabled when pending packets exceeds VHOST_MAX_PEND 128, so tx work would not be notified to run and guest would never get the interrupt it expected to re-enable the queue.

And just like what we've discussed, tx polling based adding and signaling is too early for vhost.
So this piece of code may not help and could be removed and we need
to
poll the virt-queue during zerocopy callback ( through it could be
further optimized but may not be easy).
Thanks
Shirley

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/