* [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
@ 2013-06-05 7:40 Jason Wang
2013-06-05 9:02 ` Michael S. Tsirkin
2013-06-05 13:44 ` Sergei Shtylyov
0 siblings, 2 replies; 6+ messages in thread
From: Jason Wang @ 2013-06-05 7:40 UTC (permalink / raw)
To: mst, virtualization, netdev, kvm, linux-kernel
When we decide not use zero-copy, msg.control should be set to 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/2011
ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
Call Trace:
[<ffffffff81796b73>] dump_stack+0x19/0x1e
[<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
[<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
[<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
[<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
[<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
[<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
[<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
[<ffffffff81061f46>] kthread+0xc6/0xd0
[<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
[<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
[<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2b51e23..b07d96b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -436,7 +436,8 @@ static void handle_tx(struct vhost_net *net)
kref_get(&ubufs->kref);
}
nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
- }
+ } else
+ msg.msg_control = NULL;
/* TODO: Check specific error and bomb out unless ENOBUFS? */
err = sock->ops->sendmsg(NULL, sock, &msg, len);
if (unlikely(err < 0)) {
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
2013-06-05 7:40 [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx Jason Wang
@ 2013-06-05 9:02 ` Michael S. Tsirkin
2013-06-10 21:32 ` David Miller
2013-06-05 13:44 ` Sergei Shtylyov
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-06-05 9:02 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
On Wed, Jun 05, 2013 at 03:40:46PM +0800, Jason Wang wrote:
> When we decide not use zero-copy, msg.control should be set to 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/2011
> ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
> ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
> ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
> Call Trace:
> [<ffffffff81796b73>] dump_stack+0x19/0x1e
> [<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
> [<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
> [<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
> [<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
> [<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
> [<ffffffff81061f46>] kthread+0xc6/0xd0
> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Good catch.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
This needs to go into stable as well.
> ---
> drivers/vhost/net.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 2b51e23..b07d96b 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -436,7 +436,8 @@ static void handle_tx(struct vhost_net *net)
> kref_get(&ubufs->kref);
> }
> nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
> - }
> + } else
> + msg.msg_control = NULL;
> /* TODO: Check specific error and bomb out unless ENOBUFS? */
> err = sock->ops->sendmsg(NULL, sock, &msg, len);
> if (unlikely(err < 0)) {
> --
> 1.7.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
2013-06-05 9:02 ` Michael S. Tsirkin
@ 2013-06-10 21:32 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-06-10 21:32 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, kvm, virtualization
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 5 Jun 2013 12:02:52 +0300
> On Wed, Jun 05, 2013 at 03:40:46PM +0800, Jason Wang wrote:
>> When we decide not use zero-copy, msg.control should be set to 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/2011
>> ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
>> ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
>> ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
>> Call Trace:
>> [<ffffffff81796b73>] dump_stack+0x19/0x1e
>> [<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
>> [<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
>> [<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
>> [<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
>> [<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>> [<ffffffff81061f46>] kthread+0xc6/0xd0
>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>> [<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> Good catch.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> This needs to go into stable as well.
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
2013-06-05 7:40 [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx Jason Wang
2013-06-05 9:02 ` Michael S. Tsirkin
@ 2013-06-05 13:44 ` Sergei Shtylyov
2013-06-06 3:27 ` Jason Wang
1 sibling, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2013-06-05 13:44 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
Hello.
On 05-06-2013 11:40, Jason Wang wrote:
> When we decide not use zero-copy, msg.control should be set to 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/2011
> ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
> ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
> ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
> Call Trace:
> [<ffffffff81796b73>] dump_stack+0x19/0x1e
> [<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
> [<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
> [<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
> [<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
> [<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
> [<ffffffff81061f46>] kthread+0xc6/0xd0
> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
> [<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 2b51e23..b07d96b 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -436,7 +436,8 @@ static void handle_tx(struct vhost_net *net)
> kref_get(&ubufs->kref);
> }
> nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
> - }
> + } else
You have to use {} on the *else* branch if you have it of the *if*
branch (and vice versa), according to Documentation/CodingStyle.
> + msg.msg_control = NULL;
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
2013-06-05 13:44 ` Sergei Shtylyov
@ 2013-06-06 3:27 ` Jason Wang
2013-06-06 17:12 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2013-06-06 3:27 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, virtualization, linux-kernel, kvm, mst
On 06/05/2013 09:44 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 05-06-2013 11:40, Jason Wang wrote:
>
>> When we decide not use zero-copy, msg.control should be set to 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/2011
>> ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
>> ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
>> ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
>> Call Trace:
>> [<ffffffff81796b73>] dump_stack+0x19/0x1e
>> [<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
>> [<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
>> [<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
>> [<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
>> [<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>> [<ffffffff81061f46>] kthread+0xc6/0xd0
>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>> [<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/net.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 2b51e23..b07d96b 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -436,7 +436,8 @@ static void handle_tx(struct vhost_net *net)
>> kref_get(&ubufs->kref);
>> }
>> nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
>> - }
>> + } else
>
> You have to use {} on the *else* branch if you have it of the *if*
> branch (and vice versa), according to Documentation/CodingStyle.
checkpatch.pl didn't complain this, will send v2.
Thanks
>
>> + msg.msg_control = NULL;
>
> WBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx
2013-06-06 3:27 ` Jason Wang
@ 2013-06-06 17:12 ` Sergei Shtylyov
0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2013-06-06 17:12 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
Hello.
On 06/06/2013 07:27 AM, Jason Wang wrote:
>>> When we decide not use zero-copy, msg.control should be set to 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/2011
>>> ffffffffa0198323 ffff88007c9ebd08 ffffffff81796b73 ffff88007c9ebd48
>>> ffffffff8103d66b 000000007b773e20 ffff8800779f0000 ffff8800779f43f0
>>> ffff8800779f8418 000000000000015c 0000000000000062 ffff88007c9ebd58
>>> Call Trace:
>>> [<ffffffff81796b73>] dump_stack+0x19/0x1e
>>> [<ffffffff8103d66b>] warn_slowpath_common+0x6b/0xa0
>>> [<ffffffff8103d6b5>] warn_slowpath_null+0x15/0x20
>>> [<ffffffffa0197627>] handle_tx+0x477/0x4b0 [vhost_net]
>>> [<ffffffffa0197690>] handle_tx_kick+0x10/0x20 [vhost_net]
>>> [<ffffffffa019541e>] vhost_worker+0xfe/0x1a0 [vhost_net]
>>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>>> [<ffffffffa0195320>] ? vhost_attach_cgroups_work+0x30/0x30 [vhost_net]
>>> [<ffffffff81061f46>] kthread+0xc6/0xd0
>>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>>> [<ffffffff817a1aec>] ret_from_fork+0x7c/0xb0
>>> [<ffffffff81061e80>] ? kthread_freezable_should_stop+0x70/0x70
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> drivers/vhost/net.c | 3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>> index 2b51e23..b07d96b 100644
>>> --- a/drivers/vhost/net.c
>>> +++ b/drivers/vhost/net.c
>>> @@ -436,7 +436,8 @@ static void handle_tx(struct vhost_net *net)
>>> kref_get(&ubufs->kref);
>>> }
>>> nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
>>> - }
>>> + } else
>> You have to use {} on the *else* branch if you have it of the *if*
>> branch (and vice versa), according to Documentation/CodingStyle.
> checkpatch.pl didn't complain this, will send v2.
Yes, this check is what it seems to lack still.
> Thanks
>
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-10 21:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-05 7:40 [PATCH] vhost_net: clear msg.control for non-zerocopy case during tx Jason Wang
2013-06-05 9:02 ` Michael S. Tsirkin
2013-06-10 21:32 ` David Miller
2013-06-05 13:44 ` Sergei Shtylyov
2013-06-06 3:27 ` Jason Wang
2013-06-06 17:12 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).