* [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready
@ 2013-10-14 9:56 Jason Wang
2013-10-14 9:56 ` [PATCH net 2/2] virtio-net: refill only when device is up during setting queues Jason Wang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jason Wang @ 2013-10-14 9:56 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, rusty, mst; +Cc: Jason Wang, Wanlong Gao
We're trying to re-configure the affinity unconditionally in cpu hotplug
callback. This may lead the issue during resuming from s3/s4 since
- virt queues haven't been allocated at that time.
- it's unnecessary since thaw method will re-configure the affinity.
Fix this issue by checking the config_enable and do nothing is we're not ready.
The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
(virtio-net: reset virtqueue affinity when doing cpu hotplug).
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
The patch is need for 3.8 and above.
---
drivers/net/virtio_net.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index defec2b..c4bc1cc 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1116,6 +1116,11 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
{
struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
+ mutex_lock(&vi->config_lock);
+
+ if (!vi->config_enable)
+ goto done;
+
switch(action & ~CPU_TASKS_FROZEN) {
case CPU_ONLINE:
case CPU_DOWN_FAILED:
@@ -1128,6 +1133,9 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
default:
break;
}
+
+done:
+ mutex_unlock(&vi->config_lock);
return NOTIFY_OK;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] virtio-net: refill only when device is up during setting queues
2013-10-14 9:56 [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Jason Wang
@ 2013-10-14 9:56 ` Jason Wang
2013-10-14 11:09 ` Michael S. Tsirkin
2013-10-14 11:06 ` [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Michael S. Tsirkin
2013-10-15 0:18 ` Wanlong Gao
2 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2013-10-14 9:56 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, rusty, mst; +Cc: Jason Wang
We used to schedule the refill work unconditionally after changing the
number of queues. This may lead an issue if the device is not
up. Since we only try to cancel the work in ndo_stop(), this may cause
the refill work still work after removing the device. Fix this by only
schedule the work when device is up.
The bug were introduce by commit 9b9cd8024a2882e896c65222aa421d461354e3f2.
(virtio-net: fix the race between channels setting and refill)
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
The patch were need for 3.10 and above.
---
drivers/net/virtio_net.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c4bc1cc..92f0096 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -938,7 +938,9 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
return -EINVAL;
} else {
vi->curr_queue_pairs = queue_pairs;
- schedule_delayed_work(&vi->refill, 0);
+ /* virtnet_open() will refill when device is going to up. */
+ if (dev->flags & IFF_UP)
+ schedule_delayed_work(&vi->refill, 0);
}
return 0;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready
2013-10-14 9:56 [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Jason Wang
2013-10-14 9:56 ` [PATCH net 2/2] virtio-net: refill only when device is up during setting queues Jason Wang
@ 2013-10-14 11:06 ` Michael S. Tsirkin
2013-10-15 0:18 ` Wanlong Gao
2 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 11:06 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, netdev, linux-kernel, rusty, Wanlong Gao
On Mon, Oct 14, 2013 at 05:56:34PM +0800, Jason Wang wrote:
> We're trying to re-configure the affinity unconditionally in cpu hotplug
> callback. This may lead the issue during resuming from s3/s4 since
>
> - virt queues haven't been allocated at that time.
> - it's unnecessary since thaw method will re-configure the affinity.
>
> Fix this issue by checking the config_enable and do nothing is we're not ready.
>
> The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
> (virtio-net: reset virtqueue affinity when doing cpu hotplug).
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> The patch is need for 3.8 and above.
> ---
> drivers/net/virtio_net.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index defec2b..c4bc1cc 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1116,6 +1116,11 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> {
> struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
>
> + mutex_lock(&vi->config_lock);
> +
> + if (!vi->config_enable)
> + goto done;
> +
> switch(action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> case CPU_DOWN_FAILED:
> @@ -1128,6 +1133,9 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> default:
> break;
> }
> +
> +done:
> + mutex_unlock(&vi->config_lock);
> return NOTIFY_OK;
> }
>
> --
> 1.8.1.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] virtio-net: refill only when device is up during setting queues
2013-10-14 9:56 ` [PATCH net 2/2] virtio-net: refill only when device is up during setting queues Jason Wang
@ 2013-10-14 11:09 ` Michael S. Tsirkin
2013-10-15 3:15 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-10-14 11:09 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, netdev, linux-kernel, rusty
On Mon, Oct 14, 2013 at 05:56:35PM +0800, Jason Wang wrote:
> We used to schedule the refill work unconditionally after changing the
> number of queues. This may lead an issue if the device is not
> up. Since we only try to cancel the work in ndo_stop(), this may cause
> the refill work still work after removing the device. Fix this by only
> schedule the work when device is up.
>
> The bug were introduce by commit 9b9cd8024a2882e896c65222aa421d461354e3f2.
> (virtio-net: fix the race between channels setting and refill)
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
It bothers me that we look at the flag without any
locks here.
I think we'll need to take the rtnl lock at least
on restore.
> ---
> The patch were need for 3.10 and above.
> ---
> drivers/net/virtio_net.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c4bc1cc..92f0096 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -938,7 +938,9 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> return -EINVAL;
> } else {
> vi->curr_queue_pairs = queue_pairs;
> - schedule_delayed_work(&vi->refill, 0);
> + /* virtnet_open() will refill when device is going to up. */
> + if (dev->flags & IFF_UP)
> + schedule_delayed_work(&vi->refill, 0);
> }
>
> return 0;
> --
> 1.8.1.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready
2013-10-14 9:56 [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Jason Wang
2013-10-14 9:56 ` [PATCH net 2/2] virtio-net: refill only when device is up during setting queues Jason Wang
2013-10-14 11:06 ` [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Michael S. Tsirkin
@ 2013-10-15 0:18 ` Wanlong Gao
2 siblings, 0 replies; 6+ messages in thread
From: Wanlong Gao @ 2013-10-15 0:18 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, netdev, linux-kernel, rusty, mst, Wanlong Gao
On 10/14/2013 05:56 PM, Jason Wang wrote:
> We're trying to re-configure the affinity unconditionally in cpu hotplug
> callback. This may lead the issue during resuming from s3/s4 since
>
> - virt queues haven't been allocated at that time.
> - it's unnecessary since thaw method will re-configure the affinity.
>
> Fix this issue by checking the config_enable and do nothing is we're not ready.
>
> The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
> (virtio-net: reset virtqueue affinity when doing cpu hotplug).
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Thank you.
Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> The patch is need for 3.8 and above.
> ---
> drivers/net/virtio_net.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index defec2b..c4bc1cc 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1116,6 +1116,11 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> {
> struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
>
> + mutex_lock(&vi->config_lock);
> +
> + if (!vi->config_enable)
> + goto done;
> +
> switch(action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> case CPU_DOWN_FAILED:
> @@ -1128,6 +1133,9 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> default:
> break;
> }
> +
> +done:
> + mutex_unlock(&vi->config_lock);
> return NOTIFY_OK;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] virtio-net: refill only when device is up during setting queues
2013-10-14 11:09 ` Michael S. Tsirkin
@ 2013-10-15 3:15 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2013-10-15 3:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization, netdev, linux-kernel, rusty
On 10/14/2013 07:09 PM, Michael S. Tsirkin wrote:
> On Mon, Oct 14, 2013 at 05:56:35PM +0800, Jason Wang wrote:
>> > We used to schedule the refill work unconditionally after changing the
>> > number of queues. This may lead an issue if the device is not
>> > up. Since we only try to cancel the work in ndo_stop(), this may cause
>> > the refill work still work after removing the device. Fix this by only
>> > schedule the work when device is up.
>> >
>> > The bug were introduce by commit 9b9cd8024a2882e896c65222aa421d461354e3f2.
>> > (virtio-net: fix the race between channels setting and refill)
>> >
>> > Cc: Rusty Russell <rusty@rustcorp.com.au>
>> > Cc: Michael S. Tsirkin <mst@redhat.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> It bothers me that we look at the flag without any
> locks here.
> I think we'll need to take the rtnl lock at least
> on restore.
>
True, will post v2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-15 3:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-14 9:56 [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Jason Wang
2013-10-14 9:56 ` [PATCH net 2/2] virtio-net: refill only when device is up during setting queues Jason Wang
2013-10-14 11:09 ` Michael S. Tsirkin
2013-10-15 3:15 ` Jason Wang
2013-10-14 11:06 ` [PATCH net 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready Michael S. Tsirkin
2013-10-15 0:18 ` Wanlong Gao
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).