scsi: storvsc: Properly support FC hosts

PATCH In review K. Y. Srinivasan 3 participants 8 messages Dec 23, 2015 – Jan 6, 2016

Subsystems (2)

SCSI Hyper-V/Azure core and drivers

lore

Share & tools

lore

K. Y. Srinivasan

author

2015-12-23 19:41 UTC #0 lore raw

v4 cover scsi: storvsc: Properly support FC hosts

Properly support FC hosts. Additional cleanup patches are also included.

V2: Comments from Dan Carpenter <[email protected]> and
from Johannes Thumshirn <[email protected]> addressed.

V3: Fixed build issues reported by kbuild test robot <[email protected]>
V4: Addressed comments from Hannes and James.

K. Y. Srinivasan (4):
scsi: storvsc: Fix a bug in the layout of the hv_fc_wwn_packet
scsi: storvsc: Properly support Fibre Channel devices
scsi: storvsc: Refactor the code in storvsc_channel_init()
scsi: storvsc: Tighten up the interrupt path

 drivers/scsi/storvsc_drv.c |  275 +++++++++++++++++++++++++------------------
 1 files changed, 160 insertions(+), 115 deletions(-)

Signature · 1 line

1.7.4.1

K. Y. Srinivasan

author

2015-12-23 19:41 UTC #1 lore raw

PATCH v4 1/4 scsi: storvsc: Fix a bug in the layout of the hv_fc_wwn_packet

The hv_fc_wwn_packet is exchanged over vmbus. Make the definition in Linux match the Window's definition.

Signed-off-by K. Y. Srinivasan <[email protected]>

Reviewed-by Johannes Thumshirn <[email protected]>

Reviewed-by Long Li <[email protected]>

Reviewed-by Hannes Reinecke <[email protected]>

Tested-by Alex Ng <[email protected]>

---
 drivers/scsi/storvsc_drv.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

drivers/scsi/storvsc_drv.c +2 -3

index c41f674..00bb4bd 100644
@@ -92,9 +92,8 @@ enum vstor_packet_operation {
  */
 
 struct hv_fc_wwn_packet {
-	bool	primary_active;
-	u8	reserved1;
-	u8	reserved2;
+	u8	primary_active;
+	u8	reserved1[3];
 	u8	primary_port_wwn[8];
 	u8	primary_node_wwn[8];
 	u8	secondary_port_wwn[8];

Signature · 1 line

1.7.4.1

K. Y. Srinivasan

author

2015-12-23 19:42 UTC #2 lore raw

PATCH v4 2/4 scsi: storvsc: Properly support Fibre Channel devices

For FC devices managed by this driver, atttach the appropriate transport template. This will allow us to create the appropriate sysfs files for these devices. With this we can publish the wwn for both the port and the node.

Signed-off-by K. Y. Srinivasan <[email protected]>

Reviewed-by Long Li <[email protected]>

Tested-by Alex Ng <[email protected]>

---
V2: Fixed error paths - Dan Carpenter <[email protected]>
V3: Fixed build issues reported by kbuild test robot <[email protected]>
V4: Handle configuration of SCSI_FC_ATTRS correctly (both module and built in)

 drivers/scsi/storvsc_drv.c |  181 ++++++++++++++++++++++++++++++++-----------
 1 files changed, 134 insertions(+), 47 deletions(-)

drivers/scsi/storvsc_drv.c +134 ~~-47~~

index 00bb4bd..cfbb289 100644
@@ -41,6 +41,7 @@
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_devinfo.h>
 #include <scsi/scsi_dbg.h>
+#include <scsi/scsi_transport_fc.h>
 
 /*
  * All wire protocol details (storage protocol between the guest and the host)

@@ -397,6 +398,9 @@ static int storvsc_timeout = 180;
 
 static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct scsi_transport_template *fc_transport_template;
+#endif
 
 static void storvsc_on_channel_callback(void *context);

@@ -456,6 +460,11 @@ struct storvsc_device {
 	/* Used for vsc/vsp channel reset process */
 	struct storvsc_cmd_request init_request;
 	struct storvsc_cmd_request reset_request;
+	/*
+	 * Currently active port and node names for FC devices.
+	 */
+	u64 node_name;
+	u64 port_name;
 };
 
 struct hv_host_device {

@@ -695,7 +704,26 @@ static void  handle_multichannel_storage(struct hv_device *device, int max_chns)
 	vmbus_are_subchannels_present(device->channel);
 }
 
-static int storvsc_channel_init(struct hv_device *device)
+static void cache_wwn(struct storvsc_device *stor_device,
+		      struct vstor_packet *vstor_packet)
+{
+	/*
+	 * Cache the currently active port and node ww names.
+	 */
+	if (vstor_packet->wwn_packet.primary_active) {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet->wwn_packet.primary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet->wwn_packet.primary_port_wwn);
+	} else {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet->wwn_packet.secondary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet->wwn_packet.secondary_port_wwn);
+	}
+}
+
+static int storvsc_channel_init(struct hv_device *device, bool is_fc)
 {
 	struct storvsc_device *stor_device;
 	struct storvsc_cmd_request *request;

@@ -727,19 +755,15 @@ static int storvsc_channel_init(struct hv_device *device)
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	if (ret != 0)
-		goto cleanup;
+		return ret;
 
 	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
 
 	for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {

@@ -764,18 +788,14 @@ static int storvsc_channel_init(struct hv_device *device)
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 		if (ret != 0)
-			goto cleanup;
+			return ret;
 
 		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-		if (t == 0) {
-			ret = -ETIMEDOUT;
-			goto cleanup;
-		}
+		if (t == 0)
+			return -ETIMEDOUT;
 
-		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO) {
-			ret = -EINVAL;
-			goto cleanup;
-		}
+		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO)
+			return -EINVAL;
 
 		if (vstor_packet->status == 0) {
 			vmstor_proto_version =

@@ -791,10 +811,8 @@ static int storvsc_channel_init(struct hv_device *device)
 		}
 	}
 
-	if (vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	if (vstor_packet->status != 0)
+		return -EINVAL;
 
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));

@@ -809,19 +827,15 @@ static int storvsc_channel_init(struct hv_device *device)
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
 	if (ret != 0)
-		goto cleanup;
+		return ret;
 
 	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
 	/*
 	 * Check to see if multi-channel support is there.

@@ -837,6 +851,38 @@ static int storvsc_channel_init(struct hv_device *device)
 	stor_device->max_transfer_bytes =
 		vstor_packet->storage_channel_properties.max_transfer_bytes;
 
+	if (!is_fc)
+		goto done;
+
+	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);
+
+	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.
+	 */
+	cache_wwn(stor_device, vstor_packet);
+
+done:
+
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
 	vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;

@@ -849,25 +895,19 @@ static int storvsc_channel_init(struct hv_device *device)
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
 	if (ret != 0)
-		goto cleanup;
+		return ret;
 
 	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
 	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
 	if (process_sub_channels)
 		handle_multichannel_storage(device, max_chns);
 
-
-cleanup:
 	return ret;
 }

@@ -1076,6 +1116,14 @@ static void storvsc_on_receive(struct hv_device *device,
 		schedule_work(&work->work);
 		break;
 
+	case VSTOR_OPERATION_FCHBA_DATA:
+		stor_device = get_in_stor_device(device);
+		cache_wwn(stor_device, vstor_packet);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		fc_host_node_name(stor_device->host) = stor_device->node_name;
+		fc_host_port_name(stor_device->host) = stor_device->port_name;
+#endif
+		break;
 	default:
 		break;
 	}

@@ -1131,7 +1179,8 @@ static void storvsc_on_channel_callback(void *context)
 	return;
 }
 
-static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)
+static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
+				  bool is_fc)
 {
 	struct vmstorage_channel_properties props;
 	int ret;

@@ -1148,7 +1197,7 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)
 	if (ret != 0)
 		return ret;
 
-	ret = storvsc_channel_init(device);
+	ret = storvsc_channel_init(device, is_fc);
 
 	return ret;
 }

@@ -1573,6 +1622,7 @@ static int storvsc_probe(struct hv_device *device,
 	struct Scsi_Host *host;
 	struct hv_host_device *host_dev;
 	bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
+	bool is_fc = ((dev_id->driver_data == SFC_GUID) ? true : false);
 	int target = 0;
 	struct storvsc_device *stor_device;
 	int max_luns_per_target;

@@ -1630,7 +1680,7 @@ static int storvsc_probe(struct hv_device *device,
 	hv_set_drvdata(device, stor_device);
 
 	stor_device->port_number = host->host_no;
-	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size);
+	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size, is_fc);
 	if (ret)
 		goto err_out1;

@@ -1642,6 +1692,9 @@ static int storvsc_probe(struct hv_device *device,
 		host->max_lun = STORVSC_FC_MAX_LUNS_PER_TARGET;
 		host->max_id = STORVSC_FC_MAX_TARGETS;
 		host->max_channel = STORVSC_FC_MAX_CHANNELS - 1;
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		host->transportt = fc_transport_template;
+#endif
 		break;
 
 	case SCSI_GUID:

@@ -1681,6 +1734,12 @@ static int storvsc_probe(struct hv_device *device,
 			goto err_out2;
 		}
 	}
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template) {
+		fc_host_node_name(host) = stor_device->node_name;
+		fc_host_port_name(host) = stor_device->port_name;
+	}
+#endif
 	return 0;
 
 err_out2:

@@ -1706,6 +1765,10 @@ static int storvsc_remove(struct hv_device *dev)
 	struct storvsc_device *stor_device = hv_get_drvdata(dev);
 	struct Scsi_Host *host = stor_device->host;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template)
+		fc_remove_host(host);
+#endif
 	scsi_remove_host(host);
 	storvsc_dev_remove(dev);
 	scsi_host_put(host);

@@ -1720,8 +1783,16 @@ static struct hv_driver storvsc_drv = {
 	.remove = storvsc_remove,
 };
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct fc_function_template fc_transport_functions = {
+	.show_host_node_name = 1,
+	.show_host_port_name = 1,
+};
+#endif
+
 static int __init storvsc_drv_init(void)
 {
+	int ret;
 
 	/*
 	 * Divide the ring buffer data size (which is 1 page less

@@ -1736,12 +1807,28 @@ static int __init storvsc_drv_init(void)
 		vmscsi_size_delta,
 		sizeof(u64)));
 
-	return vmbus_driver_register(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_transport_template = fc_attach_transport(&fc_transport_functions);
+	if (!fc_transport_template)
+		return -ENODEV;
+#endif
+
+	ret = vmbus_driver_register(&storvsc_drv);
+
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (ret)
+		fc_release_transport(fc_transport_template);
+#endif
+
+	return ret;
 }
 
 static void __exit storvsc_drv_exit(void)
 {
 	vmbus_driver_unregister(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_release_transport(fc_transport_template);
+#endif
 }
 
 MODULE_LICENSE("GPL");

Signature · 1 line

1.7.4.1

Hannes Reinecke

2015-12-28 12:01 UTC #3 lore raw

Re: [PATCH V4 2/4] scsi: storvsc: Properly support Fibre Channel devices

Quoted 357 lines · K. Y. Srinivasan

Signed-off-by K. Y. Srinivasan <[email protected]>

Reviewed-by Long Li <[email protected]>

Tested-by Alex Ng <[email protected]>

  drivers/scsi/storvsc_drv.c |  181 ++++++++++++++++++++++++++++++++-----------
  1 files changed, 134 insertions(+), 47 deletions(-)

drivers/scsi/storvsc_drv.c +131 ~~-47~~

index 00bb4bd..cfbb289 100644
@@ -41,6 +41,7 @@
  #include <scsi/scsi_eh.h>
  #include <scsi/scsi_devinfo.h>
  #include <scsi/scsi_dbg.h>
+#include <scsi/scsi_transport_fc.h>
 
  /*
   * All wire protocol details (storage protocol between the guest and the host)

@@ -397,6 +398,9 @@ static int storvsc_timeout = 180;
 
  static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct scsi_transport_template *fc_transport_template;
+#endif
 
  static void storvsc_on_channel_callback(void *context);

@@ -456,6 +460,11 @@ struct storvsc_device {
  	/* Used for vsc/vsp channel reset process */
  	struct storvsc_cmd_request init_request;
  	struct storvsc_cmd_request reset_request;
+	/*
+	 * Currently active port and node names for FC devices.
+	 */
+	u64 node_name;
+	u64 port_name;
  };
 
  struct hv_host_device {

@@ -695,7 +704,26 @@ static void  handle_multichannel_storage(struct hv_device *device, int max_chns)
  	vmbus_are_subchannels_present(device->channel);
  }
 
-static int storvsc_channel_init(struct hv_device *device)
+static void cache_wwn(struct storvsc_device *stor_device,
+		      struct vstor_packet *vstor_packet)
+{
+	/*
+	 * Cache the currently active port and node ww names.
+	 */
+	if (vstor_packet->wwn_packet.primary_active) {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet->wwn_packet.primary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet->wwn_packet.primary_port_wwn);
+	} else {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet->wwn_packet.secondary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet->wwn_packet.secondary_port_wwn);
+	}
+}
+
+static int storvsc_channel_init(struct hv_device *device, bool is_fc)
  {
  	struct storvsc_device *stor_device;
  	struct storvsc_cmd_request *request;

@@ -727,19 +755,15 @@ static int storvsc_channel_init(struct hv_device *device)
  			       VM_PKT_DATA_INBAND,
  			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
  	if (ret != 0)
-		goto cleanup;
+		return ret;
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
 
  	for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {

@@ -764,18 +788,14 @@ static int storvsc_channel_init(struct hv_device *device)
  			       VM_PKT_DATA_INBAND,
  			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
  		if (ret != 0)
-			goto cleanup;
+			return ret;
 
  		t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-		if (t == 0) {
-			ret = -ETIMEDOUT;
-			goto cleanup;
-		}
+		if (t == 0)
+			return -ETIMEDOUT;
 
-		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO) {
-			ret = -EINVAL;
-			goto cleanup;
-		}
+		if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO)
+			return -EINVAL;
 
  		if (vstor_packet->status == 0) {
  			vmstor_proto_version =

@@ -791,10 +811,8 @@ static int storvsc_channel_init(struct hv_device *device)
  		}
  	}
 
-	if (vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	if (vstor_packet->status != 0)
+		return -EINVAL;
 
 
  	memset(vstor_packet, 0, sizeof(struct vstor_packet));

@@ -809,19 +827,15 @@ static int storvsc_channel_init(struct hv_device *device)
  			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
  	if (ret != 0)
-		goto cleanup;
+		return ret;
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
  	/*
  	 * Check to see if multi-channel support is there.

@@ -837,6 +851,38 @@ static int storvsc_channel_init(struct hv_device *device)
  	stor_device->max_transfer_bytes =
  		vstor_packet->storage_channel_properties.max_transfer_bytes;
 
+	if (!is_fc)
+		goto done;
+
+	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);
+
+	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.
+	 */
+	cache_wwn(stor_device, vstor_packet);
+
+done:
+
  	memset(vstor_packet, 0, sizeof(struct vstor_packet));
  	vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
  	vstor_packet->flags = REQUEST_COMPLETION_FLAG;

@@ -849,25 +895,19 @@ static int storvsc_channel_init(struct hv_device *device)
  			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
  	if (ret != 0)
-		goto cleanup;
+		return ret;
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO ||
-	    vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
  	if (process_sub_channels)
  		handle_multichannel_storage(device, max_chns);
 
-
-cleanup:
  	return ret;
  }

@@ -1076,6 +1116,14 @@ static void storvsc_on_receive(struct hv_device *device,
  		schedule_work(&work->work);
  		break;
 
+	case VSTOR_OPERATION_FCHBA_DATA:
+		stor_device = get_in_stor_device(device);
+		cache_wwn(stor_device, vstor_packet);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		fc_host_node_name(stor_device->host) = stor_device->node_name;
+		fc_host_port_name(stor_device->host) = stor_device->port_name;
+#endif
+		break;
  	default:
  		break;
  	}

@@ -1131,7 +1179,8 @@ static void storvsc_on_channel_callback(void *context)
  	return;
  }
 
-static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)
+static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
+				  bool is_fc)
  {
  	struct vmstorage_channel_properties props;
  	int ret;

@@ -1148,7 +1197,7 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)
  	if (ret != 0)
  		return ret;
 
-	ret = storvsc_channel_init(device);
+	ret = storvsc_channel_init(device, is_fc);
 
  	return ret;
  }

@@ -1573,6 +1622,7 @@ static int storvsc_probe(struct hv_device *device,
  	struct Scsi_Host *host;
  	struct hv_host_device *host_dev;
  	bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
+	bool is_fc = ((dev_id->driver_data == SFC_GUID) ? true : false);
  	int target = 0;
  	struct storvsc_device *stor_device;
  	int max_luns_per_target;

@@ -1630,7 +1680,7 @@ static int storvsc_probe(struct hv_device *device,
  	hv_set_drvdata(device, stor_device);
 
  	stor_device->port_number = host->host_no;
-	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size);
+	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size, is_fc);
  	if (ret)
  		goto err_out1;

@@ -1642,6 +1692,9 @@ static int storvsc_probe(struct hv_device *device,
  		host->max_lun = STORVSC_FC_MAX_LUNS_PER_TARGET;
  		host->max_id = STORVSC_FC_MAX_TARGETS;
  		host->max_channel = STORVSC_FC_MAX_CHANNELS - 1;
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		host->transportt = fc_transport_template;
+#endif
  		break;
 
  	case SCSI_GUID:

@@ -1681,6 +1734,12 @@ static int storvsc_probe(struct hv_device *device,
  			goto err_out2;
  		}
  	}
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template) {
+		fc_host_node_name(host) = stor_device->node_name;
+		fc_host_port_name(host) = stor_device->port_name;
+	}
+#endif
  	return 0;
 
  err_out2:

@@ -1706,6 +1765,10 @@ static int storvsc_remove(struct hv_device *dev)
  	struct storvsc_device *stor_device = hv_get_drvdata(dev);
  	struct Scsi_Host *host = stor_device->host;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template)
+		fc_remove_host(host);
+#endif
  	scsi_remove_host(host);
  	storvsc_dev_remove(dev);
  	scsi_host_put(host);

@@ -1720,8 +1783,16 @@ static struct hv_driver storvsc_drv = {
  	.remove = storvsc_remove,
  };
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct fc_function_template fc_transport_functions = {
+	.show_host_node_name = 1,
+	.show_host_port_name = 1,
+};
+#endif
+
  static int __init storvsc_drv_init(void)
  {
+	int ret;
 
  	/*
  	 * Divide the ring buffer data size (which is 1 page less

@@ -1736,12 +1807,28 @@ static int __init storvsc_drv_init(void)
  		vmscsi_size_delta,
  		sizeof(u64)));
 
-	return vmbus_driver_register(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_transport_template = fc_attach_transport(&fc_transport_functions);
+	if (!fc_transport_template)
+		return -ENODEV;
+#endif
+
+	ret = vmbus_driver_register(&storvsc_drv);
+
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (ret)
+		fc_release_transport(fc_transport_template);
+#endif
+
+	return ret;
  }

drivers/scsi/storvsc_drv.c static int __init storvsc_drv_init(void) · line 1834

Quoted 10 lines

drivers/scsi/storvsc_drv.c +3 -0

 
  static void __exit storvsc_drv_exit(void)
  {
  	vmbus_driver_unregister(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_release_transport(fc_transport_template);
+#endif
  }
 
  MODULE_LICENSE("GPL");

My original question still stands: Where is the point if supporting FC devices if CONFIG_SCSI_FC_ATTRS is disabled?
I would rather disable FC support completely if that option is deselected; otherwise you'll end up with a strange cross-breed which presents FC LUNs but no attributes.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

KY Srinivasan

author

2015-12-28 17:59 UTC #4 lore raw

RE: [PATCH V4 2/4] scsi: storvsc: Properly support Fibre Channel devices

Quoted 416 lines

-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Monday, December 28, 2015 4:02 AM
To: KY Srinivasan <[email protected]>; [email protected]; linux- [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH V4 2/4] scsi: storvsc: Properly support Fibre Channel devices
Quoted 3 lines · K. Y. Srinivasan
For FC devices managed by this driver, atttach the appropriate transport template. This will allow us to create the appropriate sysfs files for these devices. With this we can publish the wwn for both the port and the
node.
Quoted 8 lines
Signed-off-by K. Y. Srinivasan <[email protected]>
Reviewed-by Long Li <[email protected]>
Tested-by Alex Ng <[email protected]>
---
V2: Fixed error paths - Dan Carpenter <[email protected]>
V3: Fixed build issues reported by kbuild test robot <[email protected]>
V4: Handle configuration of SCSI_FC_ATTRS correctly (both module
and built in)
Quoted 2 lines
  drivers/scsi/storvsc_drv.c |  181
++++++++++++++++++++++++++++++++-----------
Quoted 7 lines
  1 files changed, 134 insertions(+), 47 deletions(-)
drivers/scsi/storvsc_drv.c +0 -0
index 00bb4bd..cfbb289 100644
drivers/scsi/storvsc_drv.c line 47
Quoted 7 lines
drivers/scsi/storvsc_drv.c +1 -0
@@ -41,6 +41,7 @@
  #include <scsi/scsi_eh.h>
  #include <scsi/scsi_devinfo.h>
  #include <scsi/scsi_dbg.h>
+#include <scsi/scsi_transport_fc.h>
 
  /*
   * All wire protocol details (storage protocol between the guest and the
host)
drivers/scsi/storvsc_drv.c static void handle_multichannel_storage(struct
Quoted 23 lines
drivers/scsi/storvsc_drv.c +8 -0
@@ -397,6 +398,9 @@ static int storvsc_timeout = 180;
 
  static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct scsi_transport_template *fc_transport_template;
+#endif
 
  static void storvsc_on_channel_callback(void *context);
 
@@ -456,6 +460,11 @@ struct storvsc_device {
  	/* Used for vsc/vsp channel reset process */
  	struct storvsc_cmd_request init_request;
  	struct storvsc_cmd_request reset_request;
+	/*
+	 * Currently active port and node names for FC devices.
+	 */
+	u64 node_name;
+	u64 port_name;
  };
 
  struct hv_host_device {
@@ -695,7 +704,26 @@ static void  handle_multichannel_storage(struct
hv_device *device, int max_chns)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 32 lines
drivers/scsi/storvsc_drv.c +16 -1
  	vmbus_are_subchannels_present(device->channel);
  }
 
-static int storvsc_channel_init(struct hv_device *device)
+static void cache_wwn(struct storvsc_device *stor_device,
+		      struct vstor_packet *vstor_packet)
+{
+	/*
+	 * Cache the currently active port and node ww names.
+	 */
+	if (vstor_packet->wwn_packet.primary_active) {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet-
 wwn_packet.primary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet-
 wwn_packet.primary_port_wwn);
+	} else {
+		stor_device->node_name =
+			wwn_to_u64(vstor_packet-
 wwn_packet.secondary_node_wwn);
+		stor_device->port_name =
+			wwn_to_u64(vstor_packet-
wwn_packet.secondary_port_wwn);
drivers/scsi/storvsc_drv.c +4 -0
+	}
+}
+
+static int storvsc_channel_init(struct hv_device *device, bool is_fc)
  {
  	struct storvsc_device *stor_device;
  	struct storvsc_cmd_request *request;
@@ -727,19 +755,15 @@ static int storvsc_channel_init(struct hv_device
*device)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device · line 756
Quoted 2 lines
drivers/scsi/storvsc_drv.c +0 -0
  			       VM_PKT_DATA_INBAND,
 
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 13 lines
drivers/scsi/storvsc_drv.c +3 -5
  	if (ret != 0)
-		goto cleanup;
+		return ret;
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO
||
> -	    vstor_packet->status != 0) {
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 9 lines
drivers/scsi/storvsc_drv.c +2 -3
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
 
  	for (i = 0; i < ARRAY_SIZE(vmstor_protocols); i++) {
@@ -764,18 +788,14 @@ static int storvsc_channel_init(struct hv_device
*device)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device · line 789
Quoted 2 lines
drivers/scsi/storvsc_drv.c +0 -0
  			       VM_PKT_DATA_INBAND,
 
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 5 lines
drivers/scsi/storvsc_drv.c +1 -1
  		if (ret != 0)
-			goto cleanup;
+			return ret;
 
  		t = wait_for_completion_timeout(&request->wait_event,
5*HZ);
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 8 lines
drivers/scsi/storvsc_drv.c +2 -5
-		if (t == 0) {
-			ret = -ETIMEDOUT;
-			goto cleanup;
-		}
+		if (t == 0)
+			return -ETIMEDOUT;
 
-		if (vstor_packet->operation !=
VSTOR_OPERATION_COMPLETE_IO) {
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 4 lines
drivers/scsi/storvsc_drv.c +1 -3
-			ret = -EINVAL;
-			goto cleanup;
-		}
+		if (vstor_packet->operation !=
VSTOR_OPERATION_COMPLETE_IO)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 5 lines
drivers/scsi/storvsc_drv.c +1 -0
+			return -EINVAL;
 
  		if (vstor_packet->status == 0) {
  			vmstor_proto_version =
@@ -791,10 +811,8 @@ static int storvsc_channel_init(struct hv_device
*device)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 13 lines
drivers/scsi/storvsc_drv.c +2 -4
  		}
  	}
 
-	if (vstor_packet->status != 0) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	if (vstor_packet->status != 0)
+		return -EINVAL;
 
 
  	memset(vstor_packet, 0, sizeof(struct vstor_packet));
@@ -809,19 +827,15 @@ static int storvsc_channel_init(struct hv_device
*device)
Quoted 1 line
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
Quoted 4 lines
if (ret != 0)
drivers/scsi/storvsc_drv.c +1 -1
-		goto cleanup;
+		return ret;
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 10 lines
drivers/scsi/storvsc_drv.c +2 -4
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO
||
> -	    vstor_packet->status != 0) {
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 9 lines
drivers/scsi/storvsc_drv.c +2 -3
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
  	/*
  	 * Check to see if multi-channel support is there.
@@ -837,6 +851,38 @@ static int storvsc_channel_init(struct hv_device
*device)
Quoted 7 lines
drivers/scsi/storvsc_drv.c +3 -0
  	stor_device->max_transfer_bytes =
  		vstor_packet-
 storage_channel_properties.max_transfer_bytes;
 
+	if (!is_fc)
+		goto done;
+
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device) · line 867
Quoted 10 lines
drivers/scsi/storvsc_drv.c +10 -0
+	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);
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 9 lines
drivers/scsi/storvsc_drv.c +9 -0
+
+	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)
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 11 lines
drivers/scsi/storvsc_drv.c +9 -0
+		return -EINVAL;
+
+	/*
+	 * Cache the currently active port and node ww names.
+	 */
+	cache_wwn(stor_device, vstor_packet);
+
+done:
+
  	memset(vstor_packet, 0, sizeof(struct vstor_packet));
  	vstor_packet->operation =
VSTOR_OPERATION_END_INITIALIZATION;
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device
Quoted 2 lines
drivers/scsi/storvsc_drv.c +0 -0
  	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
@@ -849,25 +895,19 @@ static int storvsc_channel_init(struct hv_device
*device)
Quoted 1 line
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
Quoted 4 lines
if (ret != 0)
drivers/scsi/storvsc_drv.c +1 -1
-		goto cleanup;
+		return ret;
drivers/scsi/storvsc_drv.c static int storvsc_channel_init(struct hv_device *device)
Quoted 10 lines
drivers/scsi/storvsc_drv.c +2 -4
 
  	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
  	if (vstor_packet->operation != VSTOR_OPERATION_COMPLETE_IO
||
> -	    vstor_packet->status != 0) {
drivers/scsi/storvsc_drv.c static void storvsc_on_receive(struct hv_device
Quoted 15 lines
drivers/scsi/storvsc_drv.c +2 -5
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    vstor_packet->status != 0)
+		return -EINVAL;
 
  	if (process_sub_channels)
  		handle_multichannel_storage(device, max_chns);
 
-
-cleanup:
  	return ret;
  }
@@ -1076,6 +1116,14 @@ static void storvsc_on_receive(struct hv_device
*device,
drivers/scsi/storvsc_drv.c static void storvsc_on_channel_callback(void
Quoted 17 lines
drivers/scsi/storvsc_drv.c +8 -0
  		schedule_work(&work->work);
  		break;
 
+	case VSTOR_OPERATION_FCHBA_DATA:
+		stor_device = get_in_stor_device(device);
+		cache_wwn(stor_device, vstor_packet);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		fc_host_node_name(stor_device->host) = stor_device-
 node_name;
+		fc_host_port_name(stor_device->host) = stor_device-
 port_name;
+#endif
+		break;
  	default:
  		break;
  	}
drivers/scsi/storvsc_drv.c +0 -0
@@ -1131,7 +1179,8 @@ static void storvsc_on_channel_callback(void
*context)
drivers/scsi/storvsc_drv.c static int storvsc_connect_to_vsp(struct
Quoted 10 lines
drivers/scsi/storvsc_drv.c +2 -1
  	return;
  }
 
-static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size)
+static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
+				  bool is_fc)
  {
  	struct vmstorage_channel_properties props;
  	int ret;
@@ -1148,7 +1197,7 @@ static int storvsc_connect_to_vsp(struct
hv_device *device, u32 ring_size)
drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device
Quoted 36 lines
drivers/scsi/storvsc_drv.c +6 -2
  	if (ret != 0)
  		return ret;
 
-	ret = storvsc_channel_init(device);
+	ret = storvsc_channel_init(device, is_fc);
 
  	return ret;
  }
@@ -1573,6 +1622,7 @@ static int storvsc_probe(struct hv_device *device,
  	struct Scsi_Host *host;
  	struct hv_host_device *host_dev;
  	bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
+	bool is_fc = ((dev_id->driver_data == SFC_GUID) ? true : false);
  	int target = 0;
  	struct storvsc_device *stor_device;
  	int max_luns_per_target;
@@ -1630,7 +1680,7 @@ static int storvsc_probe(struct hv_device *device,
  	hv_set_drvdata(device, stor_device);
 
  	stor_device->port_number = host->host_no;
-	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size);
+	ret = storvsc_connect_to_vsp(device, storvsc_ringbuffer_size, is_fc);
  	if (ret)
  		goto err_out1;
 
@@ -1642,6 +1692,9 @@ static int storvsc_probe(struct hv_device *device,
  		host->max_lun = STORVSC_FC_MAX_LUNS_PER_TARGET;
  		host->max_id = STORVSC_FC_MAX_TARGETS;
  		host->max_channel = STORVSC_FC_MAX_CHANNELS - 1;
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+		host->transportt = fc_transport_template;
+#endif
  		break;
 
  	case SCSI_GUID:
@@ -1681,6 +1734,12 @@ static int storvsc_probe(struct hv_device
*device,
drivers/scsi/storvsc_drv.c static int storvsc_remove(struct hv_device
Quoted 13 lines
drivers/scsi/storvsc_drv.c +6 -0
  			goto err_out2;
  		}
  	}
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template) {
+		fc_host_node_name(host) = stor_device->node_name;
+		fc_host_port_name(host) = stor_device->port_name;
+	}
+#endif
  	return 0;
 
  err_out2:
@@ -1706,6 +1765,10 @@ static int storvsc_remove(struct hv_device
*dev)
Quoted 28 lines
drivers/scsi/storvsc_drv.c +12 -0
  	struct storvsc_device *stor_device = hv_get_drvdata(dev);
  	struct Scsi_Host *host = stor_device->host;
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (host->transportt == fc_transport_template)
+		fc_remove_host(host);
+#endif
  	scsi_remove_host(host);
  	storvsc_dev_remove(dev);
  	scsi_host_put(host);
@@ -1720,8 +1783,16 @@ static struct hv_driver storvsc_drv = {
  	.remove = storvsc_remove,
  };
 
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+static struct fc_function_template fc_transport_functions = {
+	.show_host_node_name = 1,
+	.show_host_port_name = 1,
+};
+#endif
+
  static int __init storvsc_drv_init(void)
  {
+	int ret;
 
  	/*
  	 * Divide the ring buffer data size (which is 1 page less
drivers/scsi/storvsc_drv.c static int __init storvsc_drv_init(void) · line 1811
Quoted 6 lines
drivers/scsi/storvsc_drv.c +2 -1
@@ -1736,12 +1807,28 @@ static int __init storvsc_drv_init(void)
  		vmscsi_size_delta,
  		sizeof(u64)));
 
-	return vmbus_driver_register(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_transport_template =
fc_attach_transport(&fc_transport_functions);
Quoted 22 lines
drivers/scsi/storvsc_drv.c +15 -0
+	if (!fc_transport_template)
+		return -ENODEV;
+#endif
+
+	ret = vmbus_driver_register(&storvsc_drv);
+
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	if (ret)
+		fc_release_transport(fc_transport_template);
+#endif
+
+	return ret;
  }
 
  static void __exit storvsc_drv_exit(void)
  {
  	vmbus_driver_unregister(&storvsc_drv);
+#if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
+	fc_release_transport(fc_transport_template);
+#endif
  }
 

Quoted 7 lines

drivers/scsi/storvsc_drv.c static int __init storvsc_drv_init(void) · line 1834
Quoted 2 lines
drivers/scsi/storvsc_drv.c +0 -0
  MODULE_LICENSE("GPL");
My original question still stands: Where is the point if supporting FC devices if CONFIG_SCSI_FC_ATTRS is disabled?
I would rather disable FC support completely if that option is deselected; otherwise you'll end up with a strange cross-breed which presents FC LUNs but no attributes.

I have been supporting FC LUNs with no attributes since Win8 and based on Feedback from a handful of customers, I have now implemented a small subset of FC attributes (this yet to be committed patch-set). Furthermore, the only attributes supported on the Windows host are the wwn for port and node. So, if CONFIG_SCSI_FC_ATTRS is disabled, in the current implementation, all we would be doing is preserving the existing behavior.

Regards,

K. Y

K. Y. Srinivasan

author

2015-12-23 19:42 UTC #5 lore raw

PATCH v4 3/4 scsi: storvsc: Refactor the code in storvsc_channel_init()

In reply to K. Y. Srinivasan

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 <[email protected]>

Reviewed-by Long Li <[email protected]>

Reviewed-by Johannes Thumshirn <[email protected]>

Reviewed-by Hannes Reinecke <[email protected]>

Tested-by Alex Ng <[email protected]>

---
V2: Fixed error paths - Dan Carpenter <[email protected]>
V4: Added Reviewed-by: Hannes Reinecke <[email protected]>

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

drivers/scsi/storvsc_drv.c +46 ~~-80~~

index cfbb289..811f276 100644
@@ -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);

Signature · 1 line

1.7.4.1

K. Y. Srinivasan

author

2015-12-23 19:42 UTC #6 lore raw

PATCH v4 4/4 scsi: storvsc: Tighten up the interrupt path

In reply to K. Y. Srinivasan

On the interrupt path, we repeatedly establish the pointer to the storvsc_device. While the compiler does inline get_in_stor_device() (and other static functions) in the call chain in the interrupt path, the compiler is repeatedly inlining the call to get_in_stor_device() each time it is invoked. The return value of get_in_stor_device() can be cached in the interrupt path since there is higher level serialization in place to ensure correct handling when the module unload races with the processing of an incoming message from the host. Optimize this code path by caching the pointer to storvsc_device and passing it as an argument.

Signed-off-by K. Y. Srinivasan <[email protected]>

Reviewed-by Long Li <[email protected]>

Reviewed-by Johannes Thumshirn <[email protected]>

Reviewed-by Hannes Reinecke <[email protected]>

Tested-by Alex Ng <[email protected]>

---
Added Reviewed-by: Hannes Reinecke <[email protected]>
Added additional details to the commit log - James Bottomley <[email protected]>

 drivers/scsi/storvsc_drv.c |   23 ++++++++---------------
 1 files changed, 8 insertions(+), 15 deletions(-)

drivers/scsi/storvsc_drv.c +8 ~~-15~~

index 811f276..41c115c 100644
@@ -945,19 +945,16 @@ static void storvsc_handle_error(struct vmscsi_request *vm_srb,
 }
 
 
-static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request)
+static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request,
+				       struct storvsc_device *stor_dev)
 {
 	struct scsi_cmnd *scmnd = cmd_request->cmd;
-	struct hv_host_device *host_dev = shost_priv(scmnd->device->host);
 	struct scsi_sense_hdr sense_hdr;
 	struct vmscsi_request *vm_srb;
 	struct Scsi_Host *host;
-	struct storvsc_device *stor_dev;
-	struct hv_device *dev = host_dev->dev;
 	u32 payload_sz = cmd_request->payload_sz;
 	void *payload = cmd_request->payload;
 
-	stor_dev = get_in_stor_device(dev);
 	host = stor_dev->host;
 
 	vm_srb = &cmd_request->vstor_packet.vm_srb;

@@ -987,14 +984,13 @@ static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request)
 		kfree(payload);
 }
 
-static void storvsc_on_io_completion(struct hv_device *device,
+static void storvsc_on_io_completion(struct storvsc_device *stor_device,
 				  struct vstor_packet *vstor_packet,
 				  struct storvsc_cmd_request *request)
 {
-	struct storvsc_device *stor_device;
 	struct vstor_packet *stor_pkt;
+	struct hv_device *device = stor_device->device;
 
-	stor_device = hv_get_drvdata(device);
 	stor_pkt = &request->vstor_packet;
 
 	/*

@@ -1049,7 +1045,7 @@ static void storvsc_on_io_completion(struct hv_device *device,
 	stor_pkt->vm_srb.data_transfer_length =
 	vstor_packet->vm_srb.data_transfer_length;
 
-	storvsc_command_completion(request);
+	storvsc_command_completion(request, stor_device);
 
 	if (atomic_dec_and_test(&stor_device->num_outstanding_req) &&
 		stor_device->drain_notify)

@@ -1058,21 +1054,19 @@ static void storvsc_on_io_completion(struct hv_device *device,
 
 }
 
-static void storvsc_on_receive(struct hv_device *device,
+static void storvsc_on_receive(struct storvsc_device *stor_device,
 			     struct vstor_packet *vstor_packet,
 			     struct storvsc_cmd_request *request)
 {
 	struct storvsc_scan_work *work;
-	struct storvsc_device *stor_device;
 
 	switch (vstor_packet->operation) {
 	case VSTOR_OPERATION_COMPLETE_IO:
-		storvsc_on_io_completion(device, vstor_packet, request);
+		storvsc_on_io_completion(stor_device, vstor_packet, request);
 		break;
 
 	case VSTOR_OPERATION_REMOVE_DEVICE:
 	case VSTOR_OPERATION_ENUMERATE_BUS:
-		stor_device = get_in_stor_device(device);
 		work = kmalloc(sizeof(struct storvsc_scan_work), GFP_ATOMIC);
 		if (!work)
 			return;

@@ -1083,7 +1077,6 @@ static void storvsc_on_receive(struct hv_device *device,
 		break;
 
 	case VSTOR_OPERATION_FCHBA_DATA:
-		stor_device = get_in_stor_device(device);
 		cache_wwn(stor_device, vstor_packet);
 #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
 		fc_host_node_name(stor_device->host) = stor_device->node_name;

@@ -1133,7 +1126,7 @@ static void storvsc_on_channel_callback(void *context)
 					vmscsi_size_delta));
 				complete(&request->wait_event);
 			} else {
-				storvsc_on_receive(device,
+				storvsc_on_receive(stor_device,
 						(struct vstor_packet *)packet,
 						request);
 			}

Signature · 1 line

1.7.4.1

Martin K. Petersen

2016-01-06 21:10 UTC #7 lore raw

In reply to K. Y. Srinivasan

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"KYS" == K Y Srinivasan <[email protected]> writes:

The template discussion appears to have lost momentum and since the concerns were minor I have applied your latest series to 4.5/scsi-queue.

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering