Re: [RHEL7.6 PATCH 22/37] Drivers: hv: vmbus: Fix a rescind issue

["Bruno E. O. Meneguele" <bmeneg@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4583 bytes --]

On 20-04, Mohammed Gamal wrote:
> The current rescind processing code will not correctly handle
> the case where the host immediately rescinds a channel that has
> been offerred. In this case, we could be blocked in the open call and
> since the channel is rescinded, the host will not respond and we could
> be blocked forever in the vmbus open call.i Fix this problem.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> (cherry picked from commit 7fa32e5ec28b1609abc0b797b58267f725fc3964)
> 
> Signed-off-by: Mohammed Gamal <mgamal@redhat.com>

Just fyi, this patch was already applied by: 

http://patchwork.lab.bos.redhat.com/patch/207318/

thus I'll drop it during this patchset integration to rhel tree.

Thanks!

> ---
>  drivers/hv/channel.c      | 10 ++++++++--
>  drivers/hv/channel_mgmt.c |  7 ++++---
>  include/linux/hyperv.h    |  1 +
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 1c967f5..1d58986 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -659,22 +659,28 @@ void vmbus_close(struct vmbus_channel *channel)
>  		 */
>  		return;
>  	}
> -	mutex_lock(&vmbus_connection.channel_mutex);
>  	/*
>  	 * Close all the sub-channels first and then close the
>  	 * primary channel.
>  	 */
>  	list_for_each_safe(cur, tmp, &channel->sc_list) {
>  		cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
> -		vmbus_close_internal(cur_channel);
>  		if (cur_channel->rescind) {
> +			wait_for_completion(&cur_channel->rescind_event);
> +			mutex_lock(&vmbus_connection.channel_mutex);
> +			vmbus_close_internal(cur_channel);
>  			hv_process_channel_removal(
>  					   cur_channel->offermsg.child_relid);
> +		} else {
> +			mutex_lock(&vmbus_connection.channel_mutex);
> +			vmbus_close_internal(cur_channel);
>  		}
> +		mutex_unlock(&vmbus_connection.channel_mutex);
>  	}
>  	/*
>  	 * Now close the primary.
>  	 */
> +	mutex_lock(&vmbus_connection.channel_mutex);
>  	vmbus_close_internal(channel);
>  	mutex_unlock(&vmbus_connection.channel_mutex);
>  }
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index ec5454f..c21020b 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -333,6 +333,7 @@ static struct vmbus_channel *alloc_channel(void)
>  		return NULL;
>  
>  	spin_lock_init(&channel->lock);
> +	init_completion(&channel->rescind_event);
>  
>  	INIT_LIST_HEAD(&channel->sc_list);
>  	INIT_LIST_HEAD(&channel->percpu_list);
> @@ -898,6 +899,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
>  	/*
>  	 * Now wait for offer handling to complete.
>  	 */
> +	vmbus_rescind_cleanup(channel);
>  	while (READ_ONCE(channel->probe_done) == false) {
>  		/*
>  		 * We wait here until any channel offer is currently
> @@ -913,7 +915,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
>  	if (channel->device_obj) {
>  		if (channel->chn_rescind_callback) {
>  			channel->chn_rescind_callback(channel);
> -			vmbus_rescind_cleanup(channel);
>  			return;
>  		}
>  		/*
> @@ -922,7 +923,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
>  		 */
>  		dev = get_device(&channel->device_obj->device);
>  		if (dev) {
> -			vmbus_rescind_cleanup(channel);
>  			vmbus_device_unregister(channel->device_obj);
>  			put_device(dev);
>  		}
> @@ -936,13 +936,14 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
>  		 * 2. Then close the primary channel.
>  		 */
>  		mutex_lock(&vmbus_connection.channel_mutex);
> -		vmbus_rescind_cleanup(channel);
>  		if (channel->state == CHANNEL_OPEN_STATE) {
>  			/*
>  			 * The channel is currently not open;
>  			 * it is safe for us to cleanup the channel.
>  			 */
>  			hv_process_channel_removal(rescind->child_relid);
> +		} else {
> +			complete(&channel->rescind_event);
>  		}
>  		mutex_unlock(&vmbus_connection.channel_mutex);
>  	}
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 1552a5f..465d5db 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -724,6 +724,7 @@ struct vmbus_channel {
>  	u8 monitor_bit;
>  
>  	bool rescind; /* got rescind msg */
> +	struct completion rescind_event;
>  
>  	u32 ringbuffer_gpadlhandle;
>  
> -- 
> 1.8.3.1
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]