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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
Reviewed-by: Long Li <longli@xxxxxxxxxxxxx>
Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
Tested-by: Alex Ng <alexng@xxxxxxxxxxxxx>
---
	V2: Fixed error paths - Dan Carpenter <dan.carpenter@xxxxxxxxxx>

  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@xxxxxxx			               +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)
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux