Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1290671 > unrolled thread

[PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init()

Started by"K. Y. Srinivasan" <kys@microsoft.com>
First post2015-12-13 20:00 +0100
Last post2015-12-18 10:00 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init() "K. Y. Srinivasan" <kys@microsoft.com> - 2015-12-13 20:00 +0100
    Re: [PATCH V3 3/4] scsi: storvsc: Refactor the code in  storvsc_channel_init() Hannes Reinecke <hare@suse.de> - 2015-12-18 10:00 +0100

#1290671 — [PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init()

From"K. Y. Srinivasan" <kys@microsoft.com>
Date2015-12-13 20:00 +0100
Subject[PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init()
Message-ID<qFk2e-3De-11@gated-at.bofh.it>
The function storvsc_channel_init() repeatedly interacts with the host to
extract various channel properties. Refactor this code to eliminate code
repetition.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Tested-by: Alex Ng <alexng@microsoft.com>
---
	V2: Fixed error paths - Dan Carpenter <dan.carpenter@oracle.com>

 drivers/scsi/storvsc_drv.c |  126 ++++++++++++++++----------------------------
 1 files changed, 46 insertions(+), 80 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 220b794..d6ca4f2 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -723,29 +723,17 @@ static void cache_wwn(struct storvsc_device *stor_device,
 	}
 }
 
-static int storvsc_channel_init(struct hv_device *device, bool is_fc)
+
+static int storvsc_execute_vstor_op(struct hv_device *device,
+				    struct storvsc_cmd_request *request,
+				    bool status_check)
 {
-	struct storvsc_device *stor_device;
-	struct storvsc_cmd_request *request;
 	struct vstor_packet *vstor_packet;
-	int ret, t, i;
-	int max_chns;
-	bool process_sub_channels = false;
-
-	stor_device = get_out_stor_device(device);
-	if (!stor_device)
-		return -ENODEV;
+	int ret, t;
 
-	request = &stor_device->init_request;
 	vstor_packet = &request->vstor_packet;
 
-	/*
-	 * Now, initiate the vsc/vsp initialization protocol on the open
-	 * channel
-	 */
-	memset(request, 0, sizeof(struct storvsc_cmd_request));
 	init_completion(&request->wait_event);
-	vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
 	ret = vmbus_sendpacket(device->channel, vstor_packet,
@@ -761,17 +749,50 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 	if (t == 0)
 		return -ETIMEDOUT;
 
+	if (!status_check)
+		return ret;
+
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
 	    vstor_packet->status != 0)
 		return -EINVAL;
 
+	return ret;
+}
+
+static int storvsc_channel_init(struct hv_device *device, bool is_fc)
+{
+	struct storvsc_device *stor_device;
+	struct storvsc_cmd_request *request;
+	struct vstor_packet *vstor_packet;
+	int ret, i;
+	int max_chns;
+	bool process_sub_channels = false;
+
+	stor_device = get_out_stor_device(device);
+	if (!stor_device)
+		return -ENODEV;
+
+	request = &stor_device->init_request;
+	vstor_packet = &request->vstor_packet;
+
+	/*
+	 * Now, initiate the vsc/vsp initialization protocol on the open
+	 * channel
+	 */
+	memset(request, 0, sizeof(struct storvsc_cmd_request));
+	vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
+	ret = storvsc_execute_vstor_op(device, request, true);
+	if (ret)
+		return ret;
+	/*
+	 * Query host supported protocol version.
+	 */
 
 	for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {
 		/* reuse the packet for version range supported */
 		memset(vstor_packet, 0, sizeof(struct vstor_packet));
 		vstor_packet->operation =
 			VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
-		vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
 		vstor_packet->version.major_minor =
 			vmstor_protocols[i].protocol_version;
@@ -780,20 +801,10 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 		 * The revision number is only used in Windows; set it to 0.
 		 */
 		vstor_packet->version.revision = 0;
-
-		ret = vmbus_sendpacket(device->channel, vstor_packet,
-			       (sizeof(struct vstor_packet) -
-				vmscsi_size_delta),
-			       (unsigned long)request,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+		ret = storvsc_execute_vstor_op(device, request, false);
 		if (ret != 0)
 			return ret;
 
-		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-		if (t == 0)
-			return -ETIMEDOUT;
-
 		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO)
 			return -EINVAL;
 
@@ -817,26 +828,10 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
 	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROPERTIES;
-	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
-
-	ret = vmbus_sendpacket(device->channel, vstor_packet,
-			       (sizeof(struct vstor_packet) -
-				vmscsi_size_delta),
-			       (unsigned long)request,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-
+	ret = storvsc_execute_vstor_op(device, request, true);
 	if (ret != 0)
 		return ret;
 
-	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0)
-		return -ETIMEDOUT;
-
-	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
-		return -EINVAL;
-
 	/*
 	 * Check to see if multi-channel support is there.
 	 * Hosts that implement protocol version of 5.1 and above
@@ -854,28 +849,15 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 	if (!is_fc)
 		goto done;
 
+	/*
+	 * For FC devices retrieve FC HBA data.
+	 */
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
 	vstor_packet->operation = VSTOR_OPERATION_FCHBA_DATA;
-	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
-
-	ret = vmbus_sendpacket(device->channel, vstor_packet,
-			       (sizeof(struct vstor_packet) -
-			       vmscsi_size_delta),
-			       (unsigned long)request,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-
+	ret = storvsc_execute_vstor_op(device, request, true);
 	if (ret != 0)
 		return ret;
 
-	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0)
-		return -ETIMEDOUT;
-
-	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
-		return -EINVAL;
-
 	/*
 	 * Cache the currently active port and node ww names.
 	 */
@@ -885,26 +867,10 @@ done:
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
 	vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
-	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
-
-	ret = vmbus_sendpacket(device->channel, vstor_packet,
-			       (sizeof(struct vstor_packet) -
-				vmscsi_size_delta),
-			       (unsigned long)request,
-			       VM_PKT_DATA_INBAND,
-			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-
+	ret = storvsc_execute_vstor_op(device, request, true);
 	if (ret != 0)
 		return ret;
 
-	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0)
-		return -ETIMEDOUT;
-
-	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0)
-		return -EINVAL;
-
 	if (process_sub_channels)
 		handle_multichannel_storage(device, max_chns);
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1294522 — Re: [PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init()

FromHannes Reinecke <hare@suse.de>
Date2015-12-18 10:00 +0100
SubjectRe: [PATCH V3 3/4] scsi: storvsc: Refactor the code in storvsc_channel_init()
Message-ID<qGZ3j-3mL-7@gated-at.bofh.it>
In reply to#1290671
On 12/13/2015 09:28 PM, K. Y. Srinivasan wrote:
> The function storvsc_channel_init() repeatedly interacts with the host to
> extract various channel properties. Refactor this code to eliminate code
> repetition.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Long Li <longli@microsoft.com>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> Tested-by: Alex Ng <alexng@microsoft.com>
> ---
> 	V2: Fixed error paths - Dan Carpenter <dan.carpenter@oracle.com>
>
>   drivers/scsi/storvsc_drv.c |  126 ++++++++++++++++----------------------------
>   1 files changed, 46 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 220b794..d6ca4f2 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -723,29 +723,17 @@ static void cache_wwn(struct storvsc_device *stor_device,
>   	}
>   }
>
> -static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> +
> +static int storvsc_execute_vstor_op(struct hv_device *device,
> +				    struct storvsc_cmd_request *request,
> +				    bool status_check)
>   {
> -	struct storvsc_device *stor_device;
> -	struct storvsc_cmd_request *request;
>   	struct vstor_packet *vstor_packet;
> -	int ret, t, i;
> -	int max_chns;
> -	bool process_sub_channels = false;
> -
> -	stor_device = get_out_stor_device(device);
> -	if (!stor_device)
> -		return -ENODEV;
> +	int ret, t;
>
> -	request = &stor_device->init_request;
>   	vstor_packet = &request->vstor_packet;
>
> -	/*
> -	 * Now, initiate the vsc/vsp initialization protocol on the open
> -	 * channel
> -	 */
> -	memset(request, 0, sizeof(struct storvsc_cmd_request));
>   	init_completion(&request->wait_event);
> -	vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
>   	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
>   	ret = vmbus_sendpacket(device->channel, vstor_packet,
> @@ -761,17 +749,50 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
>   	if (t == 0)
>   		return -ETIMEDOUT;
>
> +	if (!status_check)
> +		return ret;
> +
>   	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
>   	    vstor_packet->status != 0)
>   		return -EINVAL;
>
> +	return ret;
> +}
> +
> +static int storvsc_channel_init(struct hv_device *device, bool is_fc)
> +{
> +	struct storvsc_device *stor_device;
> +	struct storvsc_cmd_request *request;
> +	struct vstor_packet *vstor_packet;
> +	int ret, i;
> +	int max_chns;
> +	bool process_sub_channels = false;
> +
> +	stor_device = get_out_stor_device(device);
> +	if (!stor_device)
> +		return -ENODEV;
> +
> +	request = &stor_device->init_request;
> +	vstor_packet = &request->vstor_packet;
> +
> +	/*
> +	 * Now, initiate the vsc/vsp initialization protocol on the open
> +	 * channel
> +	 */
> +	memset(request, 0, sizeof(struct storvsc_cmd_request));
> +	vstor_packet->operation = VSTOR_OPERATION_BEGIN_INITIALIZATION;
> +	ret = storvsc_execute_vstor_op(device, request, true);
> +	if (ret)
> +		return ret;
> +	/*
> +	 * Query host supported protocol version.
> +	 */
>
>   	for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {
>   		/* reuse the packet for version range supported */
>   		memset(vstor_packet, 0, sizeof(struct vstor_packet));
>   		vstor_packet->operation =
>   			VSTOR_OPERATION_QUERY_PROTOCOL_VERSION;
> -		vstor_packet->flags = REQUEST_COMPLETION_FLAG;
>
>   		vstor_packet->version.major_minor =
>   			vmstor_protocols[i].protocol_version;
> @@ -780,20 +801,10 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
>   		 * The revision number is only used in Windows; set it to 0.
>   		 */
>   		vstor_packet->version.revision = 0;
> -
> -		ret = vmbus_sendpacket(device->channel, vstor_packet,
> -			       (sizeof(struct vstor_packet) -
> -				vmscsi_size_delta),
> -			       (unsigned long)request,
> -			       VM_PKT_DATA_INBAND,
> -			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> +		ret = storvsc_execute_vstor_op(device, request, false);
>   		if (ret != 0)
>   			return ret;
>
> -		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> -		if (t == 0)
> -			return -ETIMEDOUT;
> -
>   		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO)
>   			return -EINVAL;
>
> @@ -817,26 +828,10 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
>
>   	memset(vstor_packet, 0, sizeof(struct vstor_packet));
>   	vstor_packet->operation = VSTOR_OPERATION_QUERY_PROPERTIES;
> -	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> -
> -	ret = vmbus_sendpacket(device->channel, vstor_packet,
> -			       (sizeof(struct vstor_packet) -
> -				vmscsi_size_delta),
> -			       (unsigned long)request,
> -			       VM_PKT_DATA_INBAND,
> -			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> +	ret = storvsc_execute_vstor_op(device, request, true);
>   	if (ret != 0)
>   		return ret;
>
> -	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> -	if (t == 0)
> -		return -ETIMEDOUT;
> -
> -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> -	    vstor_packet->status != 0)
> -		return -EINVAL;
> -
>   	/*
>   	 * Check to see if multi-channel support is there.
>   	 * Hosts that implement protocol version of 5.1 and above
> @@ -854,28 +849,15 @@ static int storvsc_channel_init(struct hv_device *device, bool is_fc)
>   	if (!is_fc)
>   		goto done;
>
> +	/*
> +	 * For FC devices retrieve FC HBA data.
> +	 */
>   	memset(vstor_packet, 0, sizeof(struct vstor_packet));
>   	vstor_packet->operation = VSTOR_OPERATION_FCHBA_DATA;
> -	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> -
> -	ret = vmbus_sendpacket(device->channel, vstor_packet,
> -			       (sizeof(struct vstor_packet) -
> -			       vmscsi_size_delta),
> -			       (unsigned long)request,
> -			       VM_PKT_DATA_INBAND,
> -			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> +	ret = storvsc_execute_vstor_op(device, request, true);
>   	if (ret != 0)
>   		return ret;
>
> -	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> -	if (t == 0)
> -		return -ETIMEDOUT;
> -
> -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> -	    vstor_packet->status != 0)
> -		return -EINVAL;
> -
>   	/*
>   	 * Cache the currently active port and node ww names.
>   	 */
> @@ -885,26 +867,10 @@ done:
>
>   	memset(vstor_packet, 0, sizeof(struct vstor_packet));
>   	vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
> -	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
> -
> -	ret = vmbus_sendpacket(device->channel, vstor_packet,
> -			       (sizeof(struct vstor_packet) -
> -				vmscsi_size_delta),
> -			       (unsigned long)request,
> -			       VM_PKT_DATA_INBAND,
> -			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> -
> +	ret = storvsc_execute_vstor_op(device, request, true);
>   	if (ret != 0)
>   		return ret;
>
> -	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
> -	if (t == 0)
> -		return -ETIMEDOUT;
> -
> -	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
> -	    vstor_packet->status != 0)
> -		return -EINVAL;
> -
>   	if (process_sub_channels)
>   		handle_multichannel_storage(device, max_chns);
>
>
The same applies here; please make the FC support configurable both 
during configuration and during runtime.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web