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 09:12:46 -0700 |
| |
On Mon, 2012-05-21 at 08:42 -0700, Shirley Ma wrote: > 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. >
Then it could be too early for vhost to notify guest anywhere in handle_tx for zerocopy. Then we might need to remove any notification in handle_tx for zerocopy to vhost zerocopy callback instead.
Adding vhost_poll_queue in vhost zerocopy callback unconditionally would consume unnecessary cpu.
We need to think about a better solution here.
Thanks Shirley
| |