[PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[Jason Wang]

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>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: 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

[PATCH net V2 2/2] virtio-net: refill only when device is up during setting queues

[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>
---
Changes from v1: add missing rtnl_lock() in virtnet_restore().
The patch were need for 3.10 and above.
---
 drivers/net/virtio_net.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c4bc1cc..9fbdfcd 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;
@@ -1741,7 +1743,9 @@ static int virtnet_restore(struct virtio_device *vdev)
 	vi->config_enable = true;
 	mutex_unlock(&vi->config_lock);
 
+	rtnl_lock();
 	virtnet_set_queues(vi, vi->curr_queue_pairs);
+	rtnl_unlock();
 
 	return 0;
 }
-- 
1.8.1.2

Re: [PATCH net V2 2/2] virtio-net: refill only when device is up during setting queues

[David Miller]

From: Jason Wang <jasowang@redhat.com>
Date: Tue, 15 Oct 2013 11:18:59 +0800

> 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>

Applied and queued up for -stable, thanks Jason.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[Rusty Russell]

Jason Wang <jasowang@redhat.com> writes:
> 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>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> The patch is need for 3.8 and above.

Please put 'CC: stable@kernel.org # 3.8+' in the commit.

(The specification of the stable line is poor, but that seems to be one
common method).

Cheers,
Rusty.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[Michael S. Tsirkin]

On Thu, Oct 17, 2013 at 09:57:41AM +1030, Rusty Russell wrote:
> Jason Wang <jasowang@redhat.com> writes:
> > 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>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > The patch is need for 3.8 and above.
> 
> Please put 'CC: stable@kernel.org # 3.8+' in the commit.

Not if this is going in through the net tree.

> 
> (The specification of the stable line is poor, but that seems to be one
> common method).
> 
> Cheers,
> Rusty.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Oct 17, 2013 at 09:57:41AM +1030, Rusty Russell wrote:
>> Jason Wang <jasowang@redhat.com> writes:
>> > 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>
>> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> > Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>> > ---
>> > The patch is need for 3.8 and above.
>> 
>> Please put 'CC: stable@kernel.org # 3.8+' in the commit.
>
> Not if this is going in through the net tree.

WTF?  Wow, there really *is* an FAQ: https://lwn.net/Articles/561669/

DaveM is the best maintainer I've ever known, but I abhor the idea that
every subsystem has its own incompatible variant on workflow and style.

Asking people to express 'CC: stable' in words is error-prone; if Dave
wants to filter it, he's quite capable.

Rusty.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[David Miller]

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Fri, 18 Oct 2013 11:30:15 +1030

> Asking people to express 'CC: stable' in words is error-prone; if Dave
> wants to filter it, he's quite capable.

Filtering it one time is one thing.

Potentially acting on that filter 100 or so times a day...

That's completely another.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[Rusty Russell]

David Miller <davem@davemloft.net> writes:
> From: Rusty Russell <rusty@rustcorp.com.au>
> Date: Fri, 18 Oct 2013 11:30:15 +1030
>
>> Asking people to express 'CC: stable' in words is error-prone; if Dave
>> wants to filter it, he's quite capable.
>
> Filtering it one time is one thing.
>
> Potentially acting on that filter 100 or so times a day...
>
> That's completely another.

I don't see the difference between reacting to:

> The patch were need for 3.10 and above.

And:

CC: stable@kernel.org # 3.10+

Except the latter is the standard form which everyone else uses.

Do you want awk script to turn the latter into the former?  Would that
really help?

Confused,
Rusty.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[David Miller]

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Fri, 18 Oct 2013 16:47:14 +1030

> Do you want awk script to turn the latter into the former?  Would that
> really help?

Rusty in the several years I've been operating this way, you're
the first person who seems to mind it.

To be honest I sometimes just silently deal with the stable CC:,
but it's much easier if things operate as they do now, and the
burdon is on the submitter instead of me.

Re: [PATCH net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

[David Miller]

From: Jason Wang <jasowang@redhat.com>
Date: Tue, 15 Oct 2013 11:18:58 +0800

> 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>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied and queued up for -stable.