search for: handle_tx

On 2018?06?30? 00:38, Michael S. Tsirkin wrote: > On Fri, Jun 29, 2018 at 05:09:50PM +0900, Toshiaki Makita wrote: >> Under heavy load vhost busypoll may run without suppressing >> notification. For example tx zerocopy callback can push tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> c...

On 2018?06?30? 00:38, Michael S. Tsirkin wrote: > On Fri, Jun 29, 2018 at 05:09:50PM +0900, Toshiaki Makita wrote: >> Under heavy load vhost busypoll may run without suppressing >> notification. For example tx zerocopy callback can push tx work while >> handle_tx() is running, then busyloop exits due to vhost_has_work() >> condition and enables notification but immediately reenters handle_tx() >> because the pushed work was tx. In this case handle_tx() tries to >> disable notification again, but when using event_idx it by design >> c...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run withou...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run withou...

...l Viro <viro at zeniv.linux.org.uk> --- drivers/vhost/net.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 6906f76..d86cc9b 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -336,7 +336,7 @@ static void handle_tx(struct vhost_net *net) { struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX]; struct vhost_virtqueue *vq = &nvq->vq; - unsigned out, in, s; + unsigned out, in; int head; struct msghdr msg = { .msg_name = NULL, @@ -395,16 +395,17 @@ static void handle_tx(struct vho...

...l Viro <viro at zeniv.linux.org.uk> --- drivers/vhost/net.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 6906f76..d86cc9b 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -336,7 +336,7 @@ static void handle_tx(struct vhost_net *net) { struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX]; struct vhost_virtqueue *vq = &nvq->vq; - unsigned out, in, s; + unsigned out, in; int head; struct msghdr msg = { .msg_name = NULL, @@ -395,16 +395,17 @@ static void handle_tx(struct vho...

...te: > On 2018?06?30? 00:38, Michael S. Tsirkin wrote: >> On Fri, Jun 29, 2018 at 05:09:50PM +0900, Toshiaki Makita wrote: >>> Under heavy load vhost busypoll may run without suppressing >>> notification. For example tx zerocopy callback can push tx work while >>> handle_tx() is running, then busyloop exits due to vhost_has_work() >>> condition and enables notification but immediately reenters handle_tx() >>> because the pushed work was tx. In this case handle_tx() tries to >>> disable notification again, but when using event_idx it by desig...

...NULL otherwise macvtap/tap may set zerocopy callbacks which may decrease the kref of ubufs wrongly. Bug were introduced by commit cedb9bdce099206290a2bdd02ce47a7b253b6a84 (vhost-net: skip head management if no outstanding). This solves the following warnings: WARNING: at include/linux/kref.h:47 handle_tx+0x477/0x4b0 [vhost_net]() Modules linked in: vhost_net macvtap macvlan tun nfsd exportfs bridge stp llc openvswitch kvm_amd kvm bnx2 megaraid_sas [last unloaded: tun] CPU: 5 PID: 8670 Comm: vhost-8668 Not tainted 3.10.0-rc2+ #1566 Hardware name: Dell Inc. PowerEdge R715/00XHKG, BIOS 1.5.2 04/19/201...

...NULL otherwise macvtap/tap may set zerocopy callbacks which may decrease the kref of ubufs wrongly. Bug were introduced by commit cedb9bdce099206290a2bdd02ce47a7b253b6a84 (vhost-net: skip head management if no outstanding). This solves the following warnings: WARNING: at include/linux/kref.h:47 handle_tx+0x477/0x4b0 [vhost_net]() Modules linked in: vhost_net macvtap macvlan tun nfsd exportfs bridge stp llc openvswitch kvm_amd kvm bnx2 megaraid_sas [last unloaded: tun] CPU: 5 PID: 8670 Comm: vhost-8668 Not tainted 3.10.0-rc2+ #1566 Hardware name: Dell Inc. PowerEdge R715/00XHKG, BIOS 1.5.2 04/19/201...

On 2018?03?28? 12:01, haibinzhang(???) wrote: > On 2018?03?27? 19:26, Jason wrote > On 2018?03?27? 17:12, haibinzhang wrote: >>> handle_tx() will delay rx for a long time when busy tx polling udp packets >>> with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT >>> takes into account only sent-bytes but no time. >> Interesting. >> >> Looking at vhost_can_busy_poll() it tries to...

On 2018?03?28? 12:01, haibinzhang(???) wrote: > On 2018?03?27? 19:26, Jason wrote > On 2018?03?27? 17:12, haibinzhang wrote: >>> handle_tx() will delay rx for a long time when busy tx polling udp packets >>> with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT >>> takes into account only sent-bytes but no time. >> Interesting. >> >> Looking at vhost_can_busy_poll() it tries to...

...lower device driver for some reason. * upend_idx is used to track end of used idx, done_idx is used to track head * of used idx. Once lower device DMA done contiguously, we will signal KVM @@ -231,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) static void handle_tx(struct vhost_net *net) { struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX]; + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX; unsigned out, in, s; int head; struct msghdr msg = { @@ -256,7 +225,7 @@ static void handle_tx(struct vhost_net *net) wmem = atomic_rea...

...lower device driver for some reason. * upend_idx is used to track end of used idx, done_idx is used to track head * of used idx. Once lower device DMA done contiguously, we will signal KVM @@ -231,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success) static void handle_tx(struct vhost_net *net) { struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX]; + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX; unsigned out, in, s; int head; struct msghdr msg = { @@ -256,7 +225,7 @@ static void handle_tx(struct vhost_net *net) wmem = atomic_rea...

...ing in tx path"), we lock rx vq mutex > after tx vq mutex is held. This may lead deadlock so we try to lock vq > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by > one"). With this commit, we avoid the deadlock with the assumption > that handle_rx() and handle_tx() run in a same process. But this > commit remove the protection for IOTLB updating which requires the > mutex of each vq to be held. > > To solve this issue, the first step is to have a exact same lock > ordering for vhost_net. This is done through: > > - For handle_rx(), if...

...ing in tx path"), we lock rx vq mutex > after tx vq mutex is held. This may lead deadlock so we try to lock vq > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by > one"). With this commit, we avoid the deadlock with the assumption > that handle_rx() and handle_tx() run in a same process. But this > commit remove the protection for IOTLB updating which requires the > mutex of each vq to be held. > > To solve this issue, the first step is to have a exact same lock > ordering for vhost_net. This is done through: > > - For handle_rx(), if...

Under heavy load vhost busypoll may run without suppressing notification. For example tx zerocopy callback can push tx work while handle_tx() is running, then busyloop exits due to vhost_has_work() condition and enables notification but immediately reenters handle_tx() because the pushed work was tx. In this case handle_tx() tries to disable notification again, but when using event_idx it by design cannot. Then busyloop will run withou...

...ex > > > after tx vq mutex is held. This may lead deadlock so we try to lock vq > > > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by > > > one"). With this commit, we avoid the deadlock with the assumption > > > that handle_rx() and handle_tx() run in a same process. But this > > > commit remove the protection for IOTLB updating which requires the > > > mutex of each vq to be held. > > > > > > To solve this issue, the first step is to have a exact same lock > > > ordering for vhost_net. This...

...ex > > > after tx vq mutex is held. This may lead deadlock so we try to lock vq > > > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by > > > one"). With this commit, we avoid the deadlock with the assumption > > > that handle_rx() and handle_tx() run in a same process. But this > > > commit remove the protection for IOTLB updating which requires the > > > mutex of each vq to be held. > > > > > > To solve this issue, the first step is to have a exact same lock > > > ordering for vhost_net. This...

...x performance poor but does not rx. I >> guess it is because rx notification from socket is not that heavy so >> did not impact the performance even when we failed to mask the >> notification. > > I think the reason is: > > For tx, we may only process used ring under handle_tx(). Busy polling > code can not recognize this case. > For rx, we call peek_head_len() after exiting busy loop, so if the work > is for rx, it can be processed in handle_rx() immediately. Sorry but I don't see the difference. Tx busypoll calls vhost_get_vq_desc() immediately after busy...

...x performance poor but does not rx. I >> guess it is because rx notification from socket is not that heavy so >> did not impact the performance even when we failed to mask the >> notification. > > I think the reason is: > > For tx, we may only process used ring under handle_tx(). Busy polling > code can not recognize this case. > For rx, we call peek_head_len() after exiting busy loop, so if the work > is for rx, it can be processed in handle_rx() immediately. Sorry but I don't see the difference. Tx busypoll calls vhost_get_vq_desc() immediately after busy...