Re: [PATCH 7/8] scsi: storvsc: Introduce the per-storvsc_device spinlock

Re: [PATCH 7/8] scsi: storvsc: Introduce the per-storvsc_device spinlock

[Wei Liu]

Cc SCSI maintainers

This patch should go via the hyperv tree because a later patch is
dependent on it. It requires and ack from SCSI maintainers though.

Thanks,
Wei.

On Wed, Jun 17, 2020 at 06:46:41PM +0200, Andrea Parri (Microsoft) wrote:
> storvsc uses the spinlock of the hv_device's primary channel to
> serialize modifications of stor_chns[] performed by storvsc_do_io()
> and storvsc_change_target_cpu(), when it could/should use a (per-)
> storvsc_device spinlock: this change untangles the synchronization
> mechanism for the (storvsc-specific) stor_chns[] array from the
> "generic" VMBus code and data structures, clarifying the scope of
> this synchronization mechanism.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/scsi/storvsc_drv.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 072ed87286578..8ff21e69a8be8 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -462,6 +462,11 @@ struct storvsc_device {
>  	 * Mask of CPUs bound to subchannels.
>  	 */
>  	struct cpumask alloced_cpus;
> +	/*
> +	 * Serializes modifications of stor_chns[] from storvsc_do_io()
> +	 * and storvsc_change_target_cpu().
> +	 */
> +	spinlock_t lock;
>  	/* Used for vsc/vsp channel reset process */
>  	struct storvsc_cmd_request init_request;
>  	struct storvsc_cmd_request reset_request;
> @@ -639,7 +644,7 @@ static void storvsc_change_target_cpu(struct vmbus_channel *channel, u32 old,
>  		return;
>  
>  	/* See storvsc_do_io() -> get_og_chn(). */
> -	spin_lock_irqsave(&device->channel->lock, flags);
> +	spin_lock_irqsave(&stor_device->lock, flags);
>  
>  	/*
>  	 * Determines if the storvsc device has other channels assigned to
> @@ -676,7 +681,7 @@ static void storvsc_change_target_cpu(struct vmbus_channel *channel, u32 old,
>  	WRITE_ONCE(stor_device->stor_chns[new], channel);
>  	cpumask_set_cpu(new, &stor_device->alloced_cpus);
>  
> -	spin_unlock_irqrestore(&device->channel->lock, flags);
> +	spin_unlock_irqrestore(&stor_device->lock, flags);
>  }
>  
>  static void handle_sc_creation(struct vmbus_channel *new_sc)
> @@ -1433,14 +1438,14 @@ static int storvsc_do_io(struct hv_device *device,
>  			}
>  		}
>  	} else {
> -		spin_lock_irqsave(&device->channel->lock, flags);
> +		spin_lock_irqsave(&stor_device->lock, flags);
>  		outgoing_channel = stor_device->stor_chns[q_num];
>  		if (outgoing_channel != NULL) {
> -			spin_unlock_irqrestore(&device->channel->lock, flags);
> +			spin_unlock_irqrestore(&stor_device->lock, flags);
>  			goto found_channel;
>  		}
>  		outgoing_channel = get_og_chn(stor_device, q_num);
> -		spin_unlock_irqrestore(&device->channel->lock, flags);
> +		spin_unlock_irqrestore(&stor_device->lock, flags);
>  	}
>  
>  found_channel:
> @@ -1881,6 +1886,7 @@ static int storvsc_probe(struct hv_device *device,
>  	init_waitqueue_head(&stor_device->waiting_to_drain);
>  	stor_device->device = device;
>  	stor_device->host = host;
> +	spin_lock_init(&stor_device->lock);
>  	hv_set_drvdata(device, stor_device);
>  
>  	stor_device->port_number = host->host_no;
> -- 
> 2.25.1
> 

Re: [PATCH 7/8] scsi: storvsc: Introduce the per-storvsc_device spinlock

[Andrea Parri]

On Fri, Jun 19, 2020 at 04:01:36PM +0000, Wei Liu wrote:
> Cc SCSI maintainers
> 
> This patch should go via the hyperv tree because a later patch is
> dependent on it. It requires and ack from SCSI maintainers though.

Right.  Sorry for the Cc: omission...  ;-/

SCSI maintainers, please let me know if you prefer me to send you a new
series with this patch (7/8) and the later/dependent hyperv patch (8/8).

(1-6/8 of this series are hyperv-specific only and have been applied to
the hyperv tree, so this would only 7-8/8 of this series out.)

Thanks,

  Andrea

Re: [PATCH 7/8] scsi: storvsc: Introduce the per-storvsc_device spinlock

[Martin K. Petersen]

Andrea,

>> This patch should go via the hyperv tree because a later patch is
>> dependent on it. It requires and ack from SCSI maintainers though.

Looks OK to me.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 7/8] scsi: storvsc: Introduce the per-storvsc_device spinlock

[Wei Liu]

On Fri, Jun 19, 2020 at 10:58:40PM -0400, Martin K. Petersen wrote:
> 
> Andrea,
> 
> >> This patch should go via the hyperv tree because a later patch is
> >> dependent on it. It requires and ack from SCSI maintainers though.
> 
> Looks OK to me.
> 
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

Thanks Martin.

> 
> -- 
> Martin K. Petersen	Oracle Linux Engineering