2015-08-12 12:23:04

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

This fixes the recent commit:
Drivers: hv: vmbus: Further improve CPU affiliation logic

Without the fix, reloading hv_netvsc hangs the guest.

Signed-off-by: Dexuan Cui <[email protected]>
---
drivers/hv/channel_mgmt.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 2f9aead..f61bd07 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -458,6 +458,19 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
continue;
}

+ if (cpumask_weight(&primary->alloced_cpus_in_node) ==
+ cpumask_weight(cpumask_of_node(primary->numa_node))) {
+ /*
+ * We have cycled through all the CPUs in the node;
+ * reset the alloced map.
+ * This is necessary because we never clear
+ * primary->alloced_cpus_in_node in other places.
+ * We need this to "break" the loop when reloading
+ * hv_netvsc in SMP guest.
+ */
+ cpumask_clear(&primary->alloced_cpus_in_node);
+ }
+
if (!cpumask_test_cpu(cur_cpu,
&primary->alloced_cpus_in_node)) {
cpumask_set_cpu(cur_cpu,
--
2.1.0


2015-08-12 12:45:22

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

> From: devel [mailto:[email protected]] On Behalf
> Of Dexuan Cui
> Sent: Wednesday, August 12, 2015 21:49
> To: [email protected]; [email protected]; driverdev-
> [email protected]; [email protected]; [email protected];
> [email protected]; KY Srinivasan <[email protected]>
> Subject: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc
>
> This fixes the recent commit:
> Drivers: hv: vmbus: Further improve CPU affiliation logic
>
> Without the fix, reloading hv_netvsc hangs the guest.
>
> Signed-off-by: Dexuan Cui <[email protected]>
> ---
> drivers/hv/channel_mgmt.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 2f9aead..f61bd07 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -458,6 +458,19 @@ static void init_vp_index(struct vmbus_channel
> *channel, const uuid_le *type_gui
> continue;
> }
>
> + if (cpumask_weight(&primary->alloced_cpus_in_node) ==
> + cpumask_weight(cpumask_of_node(primary->numa_node))) {
> + /*
> + * We have cycled through all the CPUs in the node;
> + * reset the alloced map.
> + * This is necessary because we never clear
> + * primary->alloced_cpus_in_node in other places.
> + * We need this to "break" the loop when reloading
> + * hv_netvsc in SMP guest.
> + */
> + cpumask_clear(&primary->alloced_cpus_in_node);
> + }
> +
> if (!cpumask_test_cpu(cur_cpu,
> &primary->alloced_cpus_in_node)) {
> cpumask_set_cpu(cur_cpu,
> --

Sorry, please drop the patch.

I shouldn't simply clear primary->alloced_cpus_in_node -- I didn't realize
reloading hv_netvsc doesn't invoke init_vp_index() for the primary channel.

I'll make a V2 patch.

Thanks,
-- Dexuan

2015-08-12 13:05:57

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

On Wed, Aug 12, 2015 at 12:29:46PM +0000, Dexuan Cui wrote:
> > This fixes the recent commit:
> > Drivers: hv: vmbus: Further improve CPU affiliation logic

Since you are redoing this anyway, include the git hash so we can look
it up. In fact, just use the Fixes tag.

regards,
dan carpenter

2015-08-12 13:35:21

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Wednesday, August 12, 2015 21:06
> To: Dexuan Cui <[email protected]>
> Cc: [email protected]; [email protected]; driverdev-
> [email protected]; [email protected]; [email protected];
> [email protected]; KY Srinivasan <[email protected]>
> Subject: Re: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading
> hv_netvsc
>
> On Wed, Aug 12, 2015 at 12:29:46PM +0000, Dexuan Cui wrote:
> > > This fixes the recent commit:
> > > Drivers: hv: vmbus: Further improve CPU affiliation logic
>
> Since you are redoing this anyway, include the git hash so we can look
> it up. In fact, just use the Fixes tag.
>
> dan carpenter

Hmm, I didn't read your mail in time and sent out V2 just now... :-)

I'm working on the latest linux-next (next-20150810).
I didn't add the git hash ID because I think the hash of the patch of the
same content can be different in different trees (like Greg's tree and
linux-next)?

Thanks,
-- Dexuan

2015-08-12 14:11:34

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

On Wed, Aug 12, 2015 at 01:35:07PM +0000, Dexuan Cui wrote:
> > From: Dan Carpenter [mailto:[email protected]]
> > On Wed, Aug 12, 2015 at 12:29:46PM +0000, Dexuan Cui wrote:
> > > > This fixes the recent commit:
> > > > Drivers: hv: vmbus: Further improve CPU affiliation logic
> >
> > Since you are redoing this anyway, include the git hash so we can look
> > it up. In fact, just use the Fixes tag.
> >
> > dan carpenter
>
> Hmm, I didn't read your mail in time and sent out V2 just now... :-)
>
> I'm working on the latest linux-next (next-20150810).
> I didn't add the git hash ID because I think the hash of the patch of the
> same content can be different in different trees (like Greg's tree and
> linux-next)?

Sometimes it can change, but mostly it doesn't. In this case it didn't.

The commit hash is based on the tree, the parent, and the content of the
patch description. If we rebase, add a new signed-off-by or merge
the patch through a different tree then the hash changes.

regards,
dan carpenter

2015-08-12 15:51:02

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading hv_netvsc

> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Wednesday, August 12, 2015 22:11
> To: Dexuan Cui <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] Drivers: hv: vmbus: fix init_vp_index() for reloading
> hv_netvsc
>
> On Wed, Aug 12, 2015 at 01:35:07PM +0000, Dexuan Cui wrote:
> > > From: Dan Carpenter [mailto:[email protected]]
> > > On Wed, Aug 12, 2015 at 12:29:46PM +0000, Dexuan Cui wrote:
> > > > > This fixes the recent commit:
> > > > > Drivers: hv: vmbus: Further improve CPU affiliation logic
> > >
> > > Since you are redoing this anyway, include the git hash so we can look
> > > it up. In fact, just use the Fixes tag.
> > >
> > > dan carpenter
> >
> > Hmm, I didn't read your mail in time and sent out V2 just now... :-)
> >
> > I'm working on the latest linux-next (next-20150810).
> > I didn't add the git hash ID because I think the hash of the patch of the
> > same content can be different in different trees (like Greg's tree and
> > linux-next)?
>
> Sometimes it can change, but mostly it doesn't. In this case it didn't.
>
> The commit hash is based on the tree, the parent, and the content of the
> patch description. If we rebase, add a new signed-off-by or merge
> the patch through a different tree then the hash changes.
>
> regards,
> dan carpenter

Thanks a lot for the explanation, Dan!

I'll remember to add the hash in future.

-- Dexuan