Re: [PATCH V2 4/6] vhost_net: determine whether or not to use zerocopy at one time
Hello.
On 08/30/2013 08:29 AM, Jason Wang wrote:
Currently, even if the packet length is smaller than VHOST_GOODCOPY_LEN, if
upend_idx != done_idx we still set zcopy_used to true and rollback this choice
later. This could be avoided by determine zerocopy once by checking all
conditions at one time before.
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/net.c | 46 +++---
1 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8a6dd0d..ff60c2a 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -404,43 +404,35 @@ static void handle_tx(struct vhost_net *net)
iov_length(nvq-hdr, s), hdr_size);
break;
}
- zcopy_used = zcopy (len = VHOST_GOODCOPY_LEN ||
- nvq-upend_idx != nvq-done_idx);
+
+ zcopy_used = zcopy len = VHOST_GOODCOPY_LEN
+(nvq-upend_idx + 1) % UIO_MAXIOV != nvq-done_idx
+vhost_net_tx_select_zcopy(net);
Could you leave on a first of two lines, matching the previous style?
/* use msg_control to pass vhost zerocopy ubuf info to skb */
if (zcopy_used) {
+ struct ubuf_info *ubuf;
+ ubuf = nvq-ubuf_info + nvq-upend_idx;
+
vq-heads[nvq-upend_idx].id = head;
[...]
+ vq-heads[nvq-upend_idx].len = VHOST_DMA_IN_PROGRESS;
+ ubuf-callback = vhost_zerocopy_callback;
+ ubuf-ctx = nvq-ubufs;
+ ubuf-desc = nvq-upend_idx;
+ msg.msg_control = ubuf;
+ msg.msg_controllen = sizeof(ubuf);
'sizeof(ubuf)' where 'ubuf' is a pointer? Are you sure it shouldn't be
'sizeof(*ubuf)'?
WBR, Sergei
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V2 5/6] vhost_net: poll vhost queue after marking DMA is done
On Fri, 2013-08-30 at 12:29 +0800, Jason Wang wrote: We used to poll vhost queue before making DMA is done, this is racy if vhost thread were waked up before marking DMA is done which can result the signal to be missed. Fix this by always poll the vhost thread before DMA is done. Does this bug only exist in net-next or is it older? Should the fix go to net and stable branches? Ben. Signed-off-by: Jason Wang jasow...@redhat.com --- drivers/vhost/net.c |9 + 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index ff60c2a..d09c17c 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -308,6 +308,11 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) struct vhost_virtqueue *vq = ubufs-vq; int cnt = atomic_read(ubufs-kref.refcount); + /* set len to mark this desc buffers done DMA */ + vq-heads[ubuf-desc].len = success ? + VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN; + vhost_net_ubuf_put(ubufs); + /* * Trigger polling thread if guest stopped submitting new buffers: * in this case, the refcount after decrement will eventually reach 1 @@ -318,10 +323,6 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) */ if (cnt = 2 || !(cnt % 16)) vhost_poll_queue(vq-poll); - /* set len to mark this desc buffers done DMA */ - vq-heads[ubuf-desc].len = success ? - VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN; - vhost_net_ubuf_put(ubufs); } /* Expects to be always run from workqueue - which acts as -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] percpu ida: Switch to cpumask_t, add some comments
On Wed, 2013-08-28 at 14:36 -0700, Andrew Morton wrote: On Wed, 28 Aug 2013 14:23:58 -0700 Kent Overstreet k...@daterainc.com wrote: I found things to be quite the opposite - it took 5 minutes of staring, head-scratching, double-checking and penny-dropping before I was confident that the newly-added code actually has nothing at all to do with the current code. Putting it in the same file was misleading, and I got misled. Ok... and I could see how the fact that it currently _doesn't_ have anything to do with the existing code would be confusing... Do you think that if/when it's making use of the ida rewrite it'll be ok? Or would you still prefer to have it in a new file I'm constitutionally reluctant to ever assume that any out-of-tree code will be merged. Maybe you'll get hit by a bus, and maybe the code sucks ;) Are you sure that the two things are so tangled together that they must live in the same file? If there's some nice layering between ida and percpu_ida then perhaps such a physical separation would remain appropriate? (and if so, any preference on the naming?) percpu_ida.c? Hi Andrew, I've folded Kent's two patches from this thread into the -v4 commit, and moved the logic from idr.[c,h] to percpu_ida.[c,h] as per your above recommendation. The cpumask_t changes are working as expected thus far, and will be going out a -v5 series for you to review - signoff shortly. Thank you, --nab ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
