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

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

[Dexuan Cui]

> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
> Of Dexuan Cui
> Sent: Wednesday, August 12, 2015 21:49
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan <kys@microsoft.com>
> 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 <decui@microsoft.com>
> ---
>  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

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

[Dan Carpenter]

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

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

[Dexuan Cui]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, August 12, 2015 21:06
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; KY Srinivasan <kys@microsoft.com>
> 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

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

[Dexuan Cui]

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 <decui@microsoft.com>
---
 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

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

[Dan Carpenter]

On Wed, Aug 12, 2015 at 01:35:07PM +0000, Dexuan Cui wrote:
> > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > 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

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

[Dexuan Cui]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, August 12, 2015 22:11
> To: Dexuan Cui <decui@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> apw@canonical.com
> 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:dan.carpenter@oracle.com]
> > > 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