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

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

[James Bottomley]

On Thu, 2015-08-13 at 17:07 -0700, K. Y. Srinivasan wrote:
> From: Dexuan Cui <decui@microsoft.com>
> 
> This fixes the recent commit 3b71107d73b16074afa7658f3f0fcf837aabfe24:

Which tree is this in?  upstream linus is giving me bad object on that
id.


> Drivers: hv: vmbus: Further improve CPU affiliation logic
> 
> Without the fix, reloading hv_netvsc hangs the guest.

The reason for looking for the commit id was to see if cc to stable was
necessary, is it?

James

> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/channel_mgmt.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 3ab4753..8a4105c 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -204,6 +204,8 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
>  		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
>  		list_del(&channel->listentry);
>  		spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
> +
> +		primary_channel = channel;
>  	} else {
>  		primary_channel = channel->primary_channel;
>  		spin_lock_irqsave(&primary_channel->lock, flags);
> @@ -211,6 +213,14 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
>  		primary_channel->num_sc--;
>  		spin_unlock_irqrestore(&primary_channel->lock, flags);
>  	}
> +
> +	/*
> +	 * We need to free the bit for init_vp_index() to work in the case
> +	 * of sub-channel, when we reload drivers like hv_netvsc.
> +	 */
> +	cpumask_clear_cpu(channel->target_cpu,
> +			  &primary_channel->alloced_cpus_in_node);
> +
>  	free_channel(channel);
>  }
>  
> @@ -457,6 +467,13 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
>  			continue;
>  		}
>  
> +		/*
> +		 * NOTE: in the case of sub-channel, we clear the sub-channel
> +		 * related bit(s) in primary->alloced_cpus_in_node in
> +		 * hv_process_channel_removal(), so when we reload drivers
> +		 * like hv_netvsc in SMP guest, here we're able to re-allocate
> +		 * bit from primary->alloced_cpus_in_node.
> +		 */
>  		if (!cpumask_test_cpu(cur_cpu,
>  				&primary->alloced_cpus_in_node)) {
>  			cpumask_set_cpu(cur_cpu,

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

[Dan Carpenter]

On Thu, Aug 13, 2015 at 04:09:10PM -0700, James Bottomley wrote:
> On Thu, 2015-08-13 at 17:07 -0700, K. Y. Srinivasan wrote:
> > From: Dexuan Cui <decui@microsoft.com>
> > 
> > This fixes the recent commit 3b71107d73b16074afa7658f3f0fcf837aabfe24:
> 
> Which tree is this in?  upstream linus is giving me bad object on that
> id.
> 

It's in Greg's char-misc-next tree.  I like to have the commit id so I
can look at buggy patches.

regards,
dan carpenter

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

[K. Y. Srinivasan]

From: Dexuan Cui <decui@microsoft.com>

This fixes the recent commit 3b71107d73b16074afa7658f3f0fcf837aabfe24:
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>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 3ab4753..8a4105c 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -204,6 +204,8 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
 		list_del(&channel->listentry);
 		spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
+
+		primary_channel = channel;
 	} else {
 		primary_channel = channel->primary_channel;
 		spin_lock_irqsave(&primary_channel->lock, flags);
@@ -211,6 +213,14 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 		primary_channel->num_sc--;
 		spin_unlock_irqrestore(&primary_channel->lock, flags);
 	}
+
+	/*
+	 * We need to free the bit for init_vp_index() to work in the case
+	 * of sub-channel, when we reload drivers like hv_netvsc.
+	 */
+	cpumask_clear_cpu(channel->target_cpu,
+			  &primary_channel->alloced_cpus_in_node);
+
 	free_channel(channel);
 }
 
@@ -457,6 +467,13 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
 			continue;
 		}
 
+		/*
+		 * NOTE: in the case of sub-channel, we clear the sub-channel
+		 * related bit(s) in primary->alloced_cpus_in_node in
+		 * hv_process_channel_removal(), so when we reload drivers
+		 * like hv_netvsc in SMP guest, here we're able to re-allocate
+		 * bit from primary->alloced_cpus_in_node.
+		 */
 		if (!cpumask_test_cpu(cur_cpu,
 				&primary->alloced_cpus_in_node)) {
 			cpumask_set_cpu(cur_cpu,
-- 
1.7.4.1

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

[KY Srinivasan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3302 bytes --]



> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@HansenPartnership.com]
> Sent: Thursday, August 13, 2015 4:09 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com;
> jbottomley@parallels.com; hch@infradead.org; linux-scsi@vger.kernel.org;
> apw@canonical.com; vkuznets@redhat.com; jasowang@redhat.com;
> Dexuan Cui <decui@microsoft.com>
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: fix init_vp_index() for reloading
> hv_netvsc
> 
> On Thu, 2015-08-13 at 17:07 -0700, K. Y. Srinivasan wrote:
> > From: Dexuan Cui <decui@microsoft.com>
> >
> > This fixes the recent commit 3b71107d73b16074afa7658f3f0fcf837aabfe24:
> 
> Which tree is this in?  upstream linus is giving me bad object on that
> id.

Greg's char-misc tree.
> 
> 
> > Drivers: hv: vmbus: Further improve CPU affiliation logic
> >
> > Without the fix, reloading hv_netvsc hangs the guest.
> 
> The reason for looking for the commit id was to see if cc to stable was
> necessary, is it?
The offending patch was committed on August 5th and cc to stable was not necessary.

K. Y
> 
> James
> 
> > Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/channel_mgmt.c |   17 +++++++++++++++++
> >  1 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> > index 3ab4753..8a4105c 100644
> > --- a/drivers/hv/channel_mgmt.c
> > +++ b/drivers/hv/channel_mgmt.c
> > @@ -204,6 +204,8 @@ void hv_process_channel_removal(struct
> vmbus_channel *channel, u32 relid)
> >  		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
> >  		list_del(&channel->listentry);
> >  		spin_unlock_irqrestore(&vmbus_connection.channel_lock,
> flags);
> > +
> > +		primary_channel = channel;
> >  	} else {
> >  		primary_channel = channel->primary_channel;
> >  		spin_lock_irqsave(&primary_channel->lock, flags);
> > @@ -211,6 +213,14 @@ void hv_process_channel_removal(struct
> vmbus_channel *channel, u32 relid)
> >  		primary_channel->num_sc--;
> >  		spin_unlock_irqrestore(&primary_channel->lock, flags);
> >  	}
> > +
> > +	/*
> > +	 * We need to free the bit for init_vp_index() to work in the case
> > +	 * of sub-channel, when we reload drivers like hv_netvsc.
> > +	 */
> > +	cpumask_clear_cpu(channel->target_cpu,
> > +			  &primary_channel->alloced_cpus_in_node);
> > +
> >  	free_channel(channel);
> >  }
> >
> > @@ -457,6 +467,13 @@ static void init_vp_index(struct vmbus_channel
> *channel, const uuid_le *type_gui
> >  			continue;
> >  		}
> >
> > +		/*
> > +		 * NOTE: in the case of sub-channel, we clear the sub-channel
> > +		 * related bit(s) in primary->alloced_cpus_in_node in
> > +		 * hv_process_channel_removal(), so when we reload
> drivers
> > +		 * like hv_netvsc in SMP guest, here we're able to re-allocate
> > +		 * bit from primary->alloced_cpus_in_node.
> > +		 */
> >  		if (!cpumask_test_cpu(cur_cpu,
> >  				&primary->alloced_cpus_in_node)) {
> >  			cpumask_set_cpu(cur_cpu,
> 
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥