Messages in this thread | | | Subject | Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback | From | Shirley Ma <> | Date | Mon, 21 May 2012 08:42:52 -0700 |
| |
On Mon, 2012-05-21 at 14:05 +0800, Jason Wang wrote: > >> - 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.
So it seems we still need vhost_enable_notify() in handle_tx when there is no tx in zerocopy case.
Do you know which one is more expensive: the cost of vhost_poll_queue() in each zerocopy callback or calling vhost_enable_notify()?
Have you compared the results by removing below code in handle_tx()?
- if (unlikely(num_pends > VHOST_MAX_PEND)) { - tx_poll_start(net, sock); - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags); - break; - }
> > And just like what we've discussed, tx polling based adding and > signaling is too early for vhost.
Thanks Shirley
| |