Skip to contentSkip to conversation

scsi: storvsc: Miscellaneous enhancements and fixes

PATCH Superseded K. Y. Srinivasan 4 participants 14 messages Mar 23, 2015 – Mar 24, 2015

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover scsi: storvsc: Miscellaneous enhancements and fixes

This patch-set addresses perf issues discovered on the Azure storage stack. These patches also fix a couple of bugs.

K. Y. Srinivasan (7):
scsi: storvsc: Increase the ring buffer size
scsi: storvsc: Size the queue depth based on the ringbuffer size
scsi: storvsc: Always send on the selected outgoing channel
scsi: storvsc: Retrieve information about the capability of the
target
scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
scsi: storvsc: Don't assume that the scatterlist is not chained
scsi: storvsc: Set the tablesize based on the information given by
the host

 drivers/scsi/storvsc_drv.c |  231 ++++++++++++++++++++++++++++----------------
 1 files changed, 149 insertions(+), 82 deletions(-)
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH RESEND 1/7 scsi: storvsc: Increase the ring buffer size

Increase the default ring buffer size as this can significantly improve performance especially on high latency storage back-ends.

---
 drivers/scsi/storvsc_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/storvsc_drv.c +1 -1
index efc6e44..27fe850 100644
@@ -308,7 +308,7 @@ enum storvsc_request_type {
  * This is the end of Protocol specific defines.
  */
 
-static int storvsc_ringbuffer_size = (20 * PAGE_SIZE);
+static int storvsc_ringbuffer_size = (256 * PAGE_SIZE);
 
 module_param(storvsc_ringbuffer_size, int, S_IRUGO);
 MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH RESEND 2/7 scsi: storvsc: Size the queue depth based on the ringbuffer size

Size the queue depth based on the ringbuffer size. Also accomodate for the fact that we could have multiple channels (ringbuffers) per adaptor.

---
 drivers/scsi/storvsc_drv.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)
drivers/scsi/storvsc_drv.c +16 -11
index 27fe850..5a12897 100644
@@ -309,10 +309,15 @@ enum storvsc_request_type {
  */
 
 static int storvsc_ringbuffer_size = (256 * PAGE_SIZE);
+static u32 max_outstanding_req_per_channel;
+
+static int storvsc_vcpus_per_sub_channel = 4;
 
 module_param(storvsc_ringbuffer_size, int, S_IRUGO);
 MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
 
+module_param(storvsc_vcpus_per_sub_channel, int, S_IRUGO);
+MODULE_PARM_DESC(vcpus_per_sub_channel, "Ratio of VCPUs to subchannels");
 /*
  * Timeout in seconds for all devices managed by this driver.
  */
@@ -320,7 +325,6 @@ static int storvsc_timeout = 180;
 
 static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
-#define STORVSC_MAX_IO_REQUESTS				200
 
 static void storvsc_on_channel_callback(void *context);
 
@@ -1376,7 +1380,6 @@ static int storvsc_do_io(struct hv_device *device,
 
 static int storvsc_device_configure(struct scsi_device *sdevice)
 {
-	scsi_change_queue_depth(sdevice, STORVSC_MAX_IO_REQUESTS);
 
 	blk_queue_max_segment_size(sdevice->request_queue, PAGE_SIZE);
 
@@ -1646,7 +1649,6 @@ static struct scsi_host_template scsi_driver = {
 	.eh_timed_out =		storvsc_eh_timed_out,
 	.slave_configure =	storvsc_device_configure,
 	.cmd_per_lun =		255,
-	.can_queue =		STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,
 	.this_id =		-1,
 	/* no use setting to 0 since ll_blk_rw reset it to 1 */
 	/* currently 32 */
@@ -1686,6 +1688,7 @@ static int storvsc_probe(struct hv_device *device,
 			const struct hv_vmbus_device_id *dev_id)
 {
 	int ret;
+	int num_cpus = num_online_cpus();
 	struct Scsi_Host *host;
 	struct hv_host_device *host_dev;
 	bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
@@ -1694,6 +1697,7 @@ static int storvsc_probe(struct hv_device *device,
 	int max_luns_per_target;
 	int max_targets;
 	int max_channels;
+	int max_sub_channels = 0;
 
 	/*
 	 * Based on the windows host we are running on,
@@ -1719,12 +1723,18 @@ static int storvsc_probe(struct hv_device *device,
 		max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
 		max_targets = STORVSC_MAX_TARGETS;
 		max_channels = STORVSC_MAX_CHANNELS;
+		/*
+		 * On Windows8 and above, we support sub-channels for storage.
+		 * The number of sub-channels offerred is based on the number of
+		 * VCPUs in the guest.
+		 */
+		max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
 		break;
 	}
 
-	if (dev_id->driver_data == SFC_GUID)
-		scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
-					 STORVSC_FC_MAX_TARGETS);
+	scsi_driver.can_queue = (max_outstanding_req_per_channel *
+				 max_sub_channels + 1);
+
 	host = scsi_host_alloc(&scsi_driver,
 			       sizeof(struct hv_host_device));
 	if (!host)
@@ -1837,7 +1847,6 @@ static struct hv_driver storvsc_drv = {
 
 static int __init storvsc_drv_init(void)
 {
-	u32 max_outstanding_req_per_channel;
 
 	/*
 	 * Divide the ring buffer data size (which is 1 page less
@@ -1852,10 +1861,6 @@ static int __init storvsc_drv_init(void)
 		vmscsi_size_delta,
 		sizeof(u64)));
 
-	if (max_outstanding_req_per_channel <
-	    STORVSC_MAX_IO_REQUESTS)
-		return -EINVAL;
-
 	return vmbus_driver_register(&storvsc_drv);
 }
 
Signature · 1 line
1.7.4.1
#3 lore raw

Re: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size

Quoted 78 lines · K. Y. Srinivasan

Size the queue depth based on the ringbuffer size. Also accomodate for the fact that we could have multiple channels (ringbuffers) per adaptor.

---
 drivers/scsi/storvsc_drv.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)
drivers/scsi/storvsc_drv.c +12 -3
index 27fe850..5a12897 100644
@@ -309,10 +309,15 @@ enum storvsc_request_type {
  */
 
 static int storvsc_ringbuffer_size = (256 * PAGE_SIZE);
+static u32 max_outstanding_req_per_channel;
+
+static int storvsc_vcpus_per_sub_channel = 4;
 
 module_param(storvsc_ringbuffer_size, int, S_IRUGO);
 MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
 
+module_param(storvsc_vcpus_per_sub_channel, int, S_IRUGO);
+MODULE_PARM_DESC(vcpus_per_sub_channel, "Ratio of VCPUs to subchannels");
 /*
  * Timeout in seconds for all devices managed by this driver.
  */
@@ -320,7 +325,6 @@ static int storvsc_timeout = 180;
 
 static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
-#define STORVSC_MAX_IO_REQUESTS                                200
 
 static void storvsc_on_channel_callback(void *context);
 
@@ -1376,7 +1380,6 @@ static int storvsc_do_io(struct hv_device *device,
 
 static int storvsc_device_configure(struct scsi_device *sdevice)
 {
-       scsi_change_queue_depth(sdevice, STORVSC_MAX_IO_REQUESTS);
 
        blk_queue_max_segment_size(sdevice->request_queue, PAGE_SIZE);
 
@@ -1646,7 +1649,6 @@ static struct scsi_host_template scsi_driver = {
        .eh_timed_out =         storvsc_eh_timed_out,
        .slave_configure =      storvsc_device_configure,
        .cmd_per_lun =          255,
-       .can_queue =            STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,
        .this_id =              -1,
        /* no use setting to 0 since ll_blk_rw reset it to 1 */
        /* currently 32 */
@@ -1686,6 +1688,7 @@ static int storvsc_probe(struct hv_device *device,
                        const struct hv_vmbus_device_id *dev_id)
 {
        int ret;
+       int num_cpus = num_online_cpus();
        struct Scsi_Host *host;
        struct hv_host_device *host_dev;
        bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
@@ -1694,6 +1697,7 @@ static int storvsc_probe(struct hv_device *device,
        int max_luns_per_target;
        int max_targets;
        int max_channels;
+       int max_sub_channels = 0;
 
        /*
         * Based on the windows host we are running on,
@@ -1719,12 +1723,18 @@ static int storvsc_probe(struct hv_device *device,
                max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
                max_targets = STORVSC_MAX_TARGETS;
                max_channels = STORVSC_MAX_CHANNELS;
+               /*
+                * On Windows8 and above, we support sub-channels for storage.
+                * The number of sub-channels offerred is based on the number of
+                * VCPUs in the guest.
+                */

drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device *device, · line 1737

Quoted 10 lines
drivers/scsi/storvsc_drv.c +4 -3
+               max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
                break;
        }
 
-       if (dev_id->driver_data == SFC_GUID)
-               scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
-                                        STORVSC_FC_MAX_TARGETS);
+       scsi_driver.can_queue = (max_outstanding_req_per_channel *
+                                max_sub_channels + 1);
+

If num_online_cpus() returned 1 - 3, can_queue will be set to 1 I think. Is that desired?

Quoted 12 lines
drivers/scsi/storvsc_drv.c +0 -1
        host = scsi_host_alloc(&scsi_driver,
                               sizeof(struct hv_host_device));
        if (!host)
@@ -1837,7 +1847,6 @@ static struct hv_driver storvsc_drv = {
 
 static int __init storvsc_drv_init(void)
 {
-       u32 max_outstanding_req_per_channel;
 
        /*
         * Divide the ring buffer data size (which is 1 page less

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

Quoted 9 lines
drivers/scsi/storvsc_drv.c +0 -4
@@ -1852,10 +1861,6 @@ static int __init storvsc_drv_init(void)
                vmscsi_size_delta,
                sizeof(u64)));
 
-       if (max_outstanding_req_per_channel <
-           STORVSC_MAX_IO_REQUESTS)
-               return -EINVAL;
-
        return vmbus_driver_register(&storvsc_drv);
 }

-- vs;

#4 lore raw

RE: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size

Quoted 112 lines

-----Original Message-----
From: Venkatesh Srinivas [mailto:[email protected]]
Sent: Monday, March 23, 2015 5:23 PM
To: KY Srinivasan
Cc: [email protected]; Linux Kernel Developers List; [email protected]; [email protected]; James E.J. Bottomley; Christoph Hellwig; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size

Quoted 1 line · K. Y. Srinivasan

Size the queue depth based on the ringbuffer size. Also accomodate for

the

Quoted 13 lines

fact that we could have multiple channels (ringbuffers) per adaptor.

---
 drivers/scsi/storvsc_drv.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)
drivers/scsi/storvsc_drv.c +0 -0
index 27fe850..5a12897 100644

drivers/scsi/storvsc_drv.c enum storvsc_request_type { · line 317

Quoted 9 lines
drivers/scsi/storvsc_drv.c +3 -0
@@ -309,10 +309,15 @@ enum storvsc_request_type {
  */
 
 static int storvsc_ringbuffer_size = (256 * PAGE_SIZE);
+static u32 max_outstanding_req_per_channel;
+
+static int storvsc_vcpus_per_sub_channel = 4;
 
 module_param(storvsc_ringbuffer_size, int, S_IRUGO);
 MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size

(bytes)");

drivers/scsi/storvsc_drv.c enum storvsc_request_type { · line 319

Quoted 3 lines
drivers/scsi/storvsc_drv.c +2 -0
+module_param(storvsc_vcpus_per_sub_channel, int, S_IRUGO);
+MODULE_PARM_DESC(vcpus_per_sub_channel, "Ratio of VCPUs to

subchannels");

Quoted 12 lines
drivers/scsi/storvsc_drv.c +0 -1
 /*
  * Timeout in seconds for all devices managed by this driver.
  */
@@ -320,7 +325,6 @@ static int storvsc_timeout = 180;
 
 static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
 
-#define STORVSC_MAX_IO_REQUESTS                                200
 
 static void storvsc_on_channel_callback(void *context);
 

drivers/scsi/storvsc_drv.c static int storvsc_do_io(struct hv_device *device, · line 1384

Quoted 6 lines
drivers/scsi/storvsc_drv.c +0 -1
@@ -1376,7 +1380,6 @@ static int storvsc_do_io(struct hv_device *device,
 
 static int storvsc_device_configure(struct scsi_device *sdevice)
 {
-       scsi_change_queue_depth(sdevice, STORVSC_MAX_IO_REQUESTS);
 
        blk_queue_max_segment_size(sdevice->request_queue,

PAGE_SIZE);

drivers/scsi/storvsc_drv.c static struct scsi_host_template scsi_driver = { · line 1649

Quoted 6 lines
drivers/scsi/storvsc_drv.c +0 -1
@@ -1646,7 +1649,6 @@ static struct scsi_host_template scsi_driver = {
        .eh_timed_out =         storvsc_eh_timed_out,
        .slave_configure =      storvsc_device_configure,
        .cmd_per_lun =          255,
-       .can_queue =

STORVSC_MAX_IO_REQUESTS*STORVSC_MAX_TARGETS,

drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device

Quoted 20 lines
drivers/scsi/storvsc_drv.c +2 -0
        .this_id =              -1,
        /* no use setting to 0 since ll_blk_rw reset it to 1 */
        /* currently 32 */
@@ -1686,6 +1688,7 @@ static int storvsc_probe(struct hv_device *device,
                        const struct hv_vmbus_device_id *dev_id)
 {
        int ret;
+       int num_cpus = num_online_cpus();
        struct Scsi_Host *host;
        struct hv_host_device *host_dev;
        bool dev_is_ide = ((dev_id->driver_data == IDE_GUID) ? true : false);
@@ -1694,6 +1697,7 @@ static int storvsc_probe(struct hv_device *device,
        int max_luns_per_target;
        int max_targets;
        int max_channels;
+       int max_sub_channels = 0;
 
        /*
         * Based on the windows host we are running on,
@@ -1719,12 +1723,18 @@ static int storvsc_probe(struct hv_device

*device,

drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device *device,

Quoted 6 lines
drivers/scsi/storvsc_drv.c +3 -0
                max_luns_per_target = STORVSC_MAX_LUNS_PER_TARGET;
                max_targets = STORVSC_MAX_TARGETS;
                max_channels = STORVSC_MAX_CHANNELS;
+               /*
+                * On Windows8 and above, we support sub-channels for storage.
+                * The number of sub-channels offerred is based on the number

of

drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device *device,

Quoted 3 lines
drivers/scsi/storvsc_drv.c +3 -0
+                * VCPUs in the guest.
+                */
+               max_sub_channels = (num_cpus /

storvsc_vcpus_per_sub_channel);

drivers/scsi/storvsc_drv.c static int storvsc_probe(struct hv_device *device, · line 1737

Quoted 9 lines
drivers/scsi/storvsc_drv.c +3 -3
                break;
        }
 
-       if (dev_id->driver_data == SFC_GUID)
-               scsi_driver.can_queue = (STORVSC_MAX_IO_REQUESTS *
-                                        STORVSC_FC_MAX_TARGETS);
+       scsi_driver.can_queue = (max_outstanding_req_per_channel *
+                                max_sub_channels + 1);
+
Quoted 2 lines

If num_online_cpus() returned 1 - 3, can_queue will be set to 1 I think. Is that desired?

can_ queue will be set max_outstanding_req_per_channel in
this case. That is what is expected. We will always have the primary channel; Additionally, if the guest has more than 4 VCPUs, the host will offer Additional subchannels for each 4 VCPus in the guest. So for less than 4 VCPus in the guest, we will only have the primary channel.

Regards,

K. Y

#5 lore raw

PATCH RESEND 3/7 scsi: storvsc: Always send on the selected outgoing channel

In reply to K. Y. Srinivasan

The current code always sent packets without data on the primary channel. Properly distribute sending of packets with no data amongst all available channels. I would like to thank Long Li for noticing this problem.

---
 drivers/scsi/storvsc_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/storvsc_drv.c +1 -1
index 5a12897..be8f12e 100644
@@ -1362,7 +1362,7 @@ static int storvsc_do_io(struct hv_device *device,
 				vmscsi_size_delta),
 				(unsigned long)request);
 	} else {
-		ret = vmbus_sendpacket(device->channel, vstor_packet,
+		ret = vmbus_sendpacket(outgoing_channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
 			       (unsigned long)request,
Signature · 1 line
1.7.4.1
#6 lore raw

PATCH RESEND 4/7 scsi: storvsc: Retrieve information about the capability of the target

In reply to K. Y. Srinivasan

The storage protocol informs the guest of the I/O capabilities of the storage stack. Retrieve this information and use it in the guest.

---
 drivers/scsi/storvsc_drv.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
drivers/scsi/storvsc_drv.c +6 -0
index be8f12e..5c13eec 100644
@@ -377,6 +377,10 @@ struct storvsc_device {
 	unsigned char path_id;
 	unsigned char target_id;
 
+	/*
+	 * Max I/O, the device can support.
+	 */
+	u32   max_transfer_bytes;
 	/* Used for vsc/vsp channel reset process */
 	struct storvsc_cmd_request init_request;
 	struct storvsc_cmd_request reset_request;
@@ -974,6 +978,8 @@ static int storvsc_channel_init(struct hv_device *device)
 		    STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
 			process_sub_channels = true;
 	}
+	stor_device->max_transfer_bytes =
+		vstor_packet->storage_channel_properties.max_transfer_bytes;
 
 	memset(vstor_packet, 0, sizeof(struct vstor_packet));
 	vstor_packet->operation = VSTOR_OPERATION_END_INITIALIZATION;
Signature · 1 line
1.7.4.1
#7 lore raw

PATCH 5/7 scsi: storvsc: Fix a bug in copy_from_bounce_buffer()

In reply to K. Y. Srinivasan

We may exit this function without properly freeing up the maapings we may have acquired. Fix the bug.

---
 drivers/scsi/storvsc_drv.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
drivers/scsi/storvsc_drv.c +8 -7
index 5c13eec..88f5d79 100644
@@ -754,21 +754,22 @@ static unsigned int copy_to_bounce_buffer(struct scatterlist *orig_sgl,
 			if (bounce_sgl[j].length == PAGE_SIZE) {
 				/* full..move to next entry */
 				sg_kunmap_atomic(bounce_addr);
+				bounce_addr = 0;
 				j++;
+			}
 
-				/* if we need to use another bounce buffer */
-				if (srclen || i != orig_sgl_count - 1)
-					bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+			/* if we need to use another bounce buffer */
+			if (srclen && bounce_addr == 0)
+				bounce_addr = sg_kmap_atomic(bounce_sgl, j);
 
-			} else if (srclen == 0 && i == orig_sgl_count - 1) {
-				/* unmap the last bounce that is < PAGE_SIZE */
-				sg_kunmap_atomic(bounce_addr);
-			}
 		}
 
 		sg_kunmap_atomic(src_addr - orig_sgl[i].offset);
 	}
 
+	if (bounce_addr)
+		sg_kunmap_atomic(bounce_addr);
+
 	local_irq_restore(flags);
 
 	return total_copied;
Signature · 1 line
1.7.4.1
Reviewed-by on v1 5/7
#8 lore raw

RE: [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()

Quoted 13 lines

-----Original Message-----
From: devel [mailto:[email protected]] On Behalf Of K. Y. Srinivasan
Sent: Monday, March 23, 2015 2:07 PM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]
Subject: [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()

Quoted 4 lines

We may exit this function without properly freeing up the maapings we may have acquired. Fix the bug.

Quoted 50 lines
---
 drivers/scsi/storvsc_drv.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
drivers/scsi/storvsc_drv.c index renamed from drivers/scsi/storvsc_drv.c +0 -0

5c13eec..88f5d79 100644

drivers/scsi/storvsc_drv.c +8 -8
@@ -754,21 +754,22 @@ static unsigned int copy_to_bounce_buffer(struct
 scatterlist *orig_sgl,
 			if (bounce_sgl[j].length == PAGE_SIZE) {
 				/* full..move to next entry */
 				sg_kunmap_atomic(bounce_addr);
+				bounce_addr = 0;
 				j++;
+			}
 
-				/* if we need to use another bounce buffer */
-				if (srclen || i != orig_sgl_count - 1)
-					bounce_addr =
 sg_kmap_atomic(bounce_sgl,j);
+			/* if we need to use another bounce buffer */
+			if (srclen && bounce_addr == 0)
+				bounce_addr = sg_kmap_atomic(bounce_sgl,
 j);
 
-			} else if (srclen == 0 && i == orig_sgl_count - 1) {
-				/* unmap the last bounce that is < PAGE_SIZE
 */
-				sg_kunmap_atomic(bounce_addr);
-			}
 		}
 
 		sg_kunmap_atomic(src_addr - orig_sgl[i].offset);
 	}
 
+	if (bounce_addr)
+		sg_kunmap_atomic(bounce_addr);
+
 	local_irq_restore(flags);
 
 	return total_copied;
--

1.7.4.1

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

#9 lore raw

PATCH v2 7/7 scsi: storvsc: Set the tablesize based on the information given by the host

In reply to K. Y. Srinivasan

Set the tablesize based on the information given by the host.

---
v2: Rebase the code and enable clustering

 drivers/scsi/storvsc_drv.c |   89 +++++++++++++++++++++++++++++++------------
 1 files changed, 64 insertions(+), 25 deletions(-)
drivers/scsi/storvsc_drv.c +64 -25
index a599677..71871f2 100644
@@ -351,7 +351,10 @@ struct storvsc_cmd_request {
 	/* Synchronize the request/response if needed */
 	struct completion wait_event;
 
-	struct hv_multipage_buffer data_buffer;
+	struct vmbus_channel_packet_multipage_buffer mpb;
+	struct vmbus_packet_mpb_array *payload;
+	u32 payload_sz;
+
 	struct vstor_packet vstor_packet;
 };
 
@@ -1133,10 +1136,14 @@ static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request)
 					 sense_hdr.ascq);
 
 	scsi_set_resid(scmnd,
-		cmd_request->data_buffer.len -
+		cmd_request->payload->range.len -
 		vm_srb->data_transfer_length);
 
 	scmnd->scsi_done(scmnd);
+
+	if (cmd_request->payload_sz >
+		sizeof(struct vmbus_channel_packet_multipage_buffer))
+		kfree(cmd_request->payload);
 }
 
 static void storvsc_on_io_completion(struct hv_device *device,
@@ -1338,7 +1345,7 @@ static int storvsc_dev_remove(struct hv_device *device)
 }
 
 static int storvsc_do_io(struct hv_device *device,
-			      struct storvsc_cmd_request *request)
+			 struct storvsc_cmd_request *request)
 {
 	struct storvsc_device *stor_device;
 	struct vstor_packet *vstor_packet;
@@ -1370,13 +1377,14 @@ static int storvsc_do_io(struct hv_device *device,
 
 
 	vstor_packet->vm_srb.data_transfer_length =
-	request->data_buffer.len;
+	request->payload->range.len;
 
 	vstor_packet->operation = VSTOR_OPERATION_EXECUTE_SRB;
 
-	if (request->data_buffer.len) {
-		ret = vmbus_sendpacket_multipagebuffer(outgoing_channel,
-				&request->data_buffer,
+	if (request->payload->range.len) {
+
+		ret = vmbus_sendpacket_mpb_desc(outgoing_channel,
+				request->payload, request->payload_sz,
 				vstor_packet,
 				(sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
@@ -1550,6 +1558,10 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	unsigned int sg_count = 0;
 	struct vmscsi_request *vm_srb;
 	struct scatterlist *cur_sgl;
+	struct vmbus_packet_mpb_array  *payload;
+	u32 payload_sz;
+	u32 pfn_cnt;
+	u32 length;
 
 	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
 		/*
@@ -1603,48 +1615,72 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 
 	memcpy(vm_srb->cdb, scmnd->cmnd, vm_srb->cdb_length);
 
-	cmd_request->data_buffer.len = scsi_bufflen(scmnd);
-	if (scsi_sg_count(scmnd)) {
-		sgl = (struct scatterlist *)scsi_sglist(scmnd);
-		sg_count = scsi_sg_count(scmnd);
+	sgl = (struct scatterlist *)scsi_sglist(scmnd);
+	sg_count = scsi_sg_count(scmnd);
 
+	length = scsi_bufflen(scmnd);
+	payload = (struct vmbus_packet_mpb_array *)&cmd_request->mpb;
+	payload_sz = sizeof(cmd_request->mpb);
+
+	if (sg_count) {
 		/* check if we need to bounce the sgl */
 		if (do_bounce_buffer(sgl, scsi_sg_count(scmnd)) != -1) {
 			cmd_request->bounce_sgl =
-				create_bounce_buffer(sgl, scsi_sg_count(scmnd),
-						     scsi_bufflen(scmnd),
+				create_bounce_buffer(sgl, sg_count,
+						     length,
 						     vm_srb->data_in);
 			if (!cmd_request->bounce_sgl)
 				return SCSI_MLQUEUE_HOST_BUSY;
 
 			cmd_request->bounce_sgl_count =
-				ALIGN(scsi_bufflen(scmnd), PAGE_SIZE) >>
-					PAGE_SHIFT;
+				ALIGN(length, PAGE_SIZE) >> PAGE_SHIFT;
 
 			if (vm_srb->data_in == WRITE_TYPE)
 				copy_to_bounce_buffer(sgl,
-					cmd_request->bounce_sgl,
-					scsi_sg_count(scmnd));
+					cmd_request->bounce_sgl, sg_count);
 
 			sgl = cmd_request->bounce_sgl;
 			sg_count = cmd_request->bounce_sgl_count;
 		}
 
-		cmd_request->data_buffer.offset = sgl[0].offset;
+		pfn_cnt = DIV_ROUND_UP(sgl[0].offset + length, PAGE_SIZE);
+
+		if (pfn_cnt > MAX_PAGE_BUFFER_COUNT) {
+
+			payload_sz = (pfn_cnt * sizeof(void *) +
+				      sizeof(struct vmbus_packet_mpb_array));
+			payload = kmalloc(payload_sz, GFP_ATOMIC);
+			if (!payload) {
+				if (cmd_request->bounce_sgl_count)
+					destroy_bounce_buffer(
+					cmd_request->bounce_sgl,
+					cmd_request->bounce_sgl_count);
+
+					return SCSI_MLQUEUE_DEVICE_BUSY;
+			}
+		}
+
+		payload->range.len = length;
+		payload->range.offset = sgl[0].offset;
+
 		cur_sgl = sgl;
 		for (i = 0; i < sg_count; i++) {
-			cmd_request->data_buffer.pfn_array[i] =
+			payload->range.pfn_array[i] =
 				page_to_pfn(sg_page((cur_sgl)));
 			cur_sgl = sg_next(cur_sgl);
 		}
 
 	} else if (scsi_sglist(scmnd)) {
-		cmd_request->data_buffer.offset =
+		payload->range.len = length;
+		payload->range.offset =
 			virt_to_phys(scsi_sglist(scmnd)) & (PAGE_SIZE-1);
-		cmd_request->data_buffer.pfn_array[0] =
+		payload->range.pfn_array[0] =
 			virt_to_phys(scsi_sglist(scmnd)) >> PAGE_SHIFT;
 	}
 
+	cmd_request->payload = payload;
+	cmd_request->payload_sz = payload_sz;
+
 	/* Invokes the vsc to start an IO */
 	ret = storvsc_do_io(dev, cmd_request);
 
@@ -1673,10 +1709,7 @@ static struct scsi_host_template scsi_driver = {
 	.slave_configure =	storvsc_device_configure,
 	.cmd_per_lun =		255,
 	.this_id =		-1,
-	/* no use setting to 0 since ll_blk_rw reset it to 1 */
-	/* currently 32 */
-	.sg_tablesize =		MAX_MULTIPAGE_BUFFER_COUNT,
-	.use_clustering =	DISABLE_CLUSTERING,
+	.use_clustering =	ENABLE_CLUSTERING,
 	/* Make sure we dont get a sg segment crosses a page boundary */
 	.dma_boundary =		PAGE_SIZE-1,
 	.no_write_same =	1,
@@ -1813,6 +1846,12 @@ static int storvsc_probe(struct hv_device *device,
 	/* max cmd length */
 	host->max_cmd_len = STORVSC_MAX_CMD_LEN;
 
+	/*
+	 * set the table size based on the info we got
+	 * from the host.
+	 */
+	host->sg_tablesize = (stor_device->max_transfer_bytes >> PAGE_SHIFT);
+
 	/* Register the HBA and start the scsi bus scan */
 	ret = scsi_add_host(host, &device->device);
 	if (ret != 0)
Signature · 1 line
1.7.4.1
#10 lore raw

PATCH 6/7 scsi: storvsc: Don't assume that the scatterlist is not chained

In reply to K. Y. Srinivasan

The current code assumes that the scatterlists presented are not chained. Fix the code to not make this assumption.

---
 drivers/scsi/storvsc_drv.c |   98 +++++++++++++++++++++++++------------------
 1 files changed, 57 insertions(+), 41 deletions(-)
drivers/scsi/storvsc_drv.c +57 -41
index 88f5d79..a599677 100644
@@ -626,19 +626,6 @@ cleanup:
 	return NULL;
 }
 
-/* Disgusting wrapper functions */
-static inline unsigned long sg_kmap_atomic(struct scatterlist *sgl, int idx)
-{
-	void *addr = kmap_atomic(sg_page(sgl + idx));
-	return (unsigned long)addr;
-}
-
-static inline void sg_kunmap_atomic(unsigned long addr)
-{
-	kunmap_atomic((void *)addr);
-}
-
-
 /* Assume the original sgl has enough room */
 static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 					    struct scatterlist *bounce_sgl,
@@ -653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 	unsigned long bounce_addr = 0;
 	unsigned long dest_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_dest_sgl;
+	struct scatterlist *cur_src_sgl;
 
 	local_irq_save(flags);
-
+	cur_dest_sgl = orig_sgl;
+	cur_src_sgl = bounce_sgl;
 	for (i = 0; i < orig_sgl_count; i++) {
-		dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		dest_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_dest_sgl)) +
+				cur_dest_sgl->offset;
 		dest = dest_addr;
 		destlen = orig_sgl[i].length;
+		destlen = cur_dest_sgl->length;
 
 		if (bounce_addr == 0)
-			bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+			bounce_addr = (unsigned long)kmap_atomic(
+							sg_page(cur_src_sgl));
 
 		while (destlen) {
-			src = bounce_addr + bounce_sgl[j].offset;
-			srclen = bounce_sgl[j].length - bounce_sgl[j].offset;
+			src = bounce_addr + cur_src_sgl->offset;
+			srclen = cur_src_sgl->length - cur_src_sgl->offset;
 
 			copylen = min(srclen, destlen);
 			memcpy((void *)dest, (void *)src, copylen);
 
 			total_copied += copylen;
-			bounce_sgl[j].offset += copylen;
+			cur_src_sgl->offset += copylen;
 			destlen -= copylen;
 			dest += copylen;
 
-			if (bounce_sgl[j].offset == bounce_sgl[j].length) {
+			if (cur_src_sgl->offset == cur_src_sgl->length) {
 				/* full */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 				j++;
 
 				/*
@@ -692,21 +686,27 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 					/*
 					 * We are done; cleanup and return.
 					 */
-					sg_kunmap_atomic(dest_addr - orig_sgl[i].offset);
+					kunmap_atomic((void *)(dest_addr -
+						cur_dest_sgl->offset));
 					local_irq_restore(flags);
 					return total_copied;
 				}
 
 				/* if we need to use another bounce buffer */
-				if (destlen || i != orig_sgl_count - 1)
-					bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+				if (destlen || i != orig_sgl_count - 1) {
+					cur_src_sgl = sg_next(cur_src_sgl);
+					bounce_addr = (unsigned long)
+							kmap_atomic(
+							sg_page(cur_src_sgl));
+				}
 			} else if (destlen == 0 && i == orig_sgl_count - 1) {
 				/* unmap the last bounce that is < PAGE_SIZE */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 			}
 		}
 
-		sg_kunmap_atomic(dest_addr - orig_sgl[i].offset);
+		kunmap_atomic((void *)(dest_addr - cur_dest_sgl->offset));
+		cur_dest_sgl = sg_next(cur_dest_sgl);
 	}
 
 	local_irq_restore(flags);
@@ -727,48 +727,61 @@ static unsigned int copy_to_bounce_buffer(struct scatterlist *orig_sgl,
 	unsigned long bounce_addr = 0;
 	unsigned long src_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_src_sgl;
+	struct scatterlist *cur_dest_sgl;
 
 	local_irq_save(flags);
 
+	cur_src_sgl = orig_sgl;
+	cur_dest_sgl = bounce_sgl;
+
 	for (i = 0; i < orig_sgl_count; i++) {
-		src_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		src_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_src_sgl)) +
+				cur_src_sgl->offset;
 		src = src_addr;
-		srclen = orig_sgl[i].length;
+		srclen = cur_src_sgl->length;
 
 		if (bounce_addr == 0)
-			bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+			bounce_addr = (unsigned long)
+					kmap_atomic(sg_page(cur_dest_sgl));
 
 		while (srclen) {
 			/* assume bounce offset always == 0 */
-			dest = bounce_addr + bounce_sgl[j].length;
-			destlen = PAGE_SIZE - bounce_sgl[j].length;
+			dest = bounce_addr + cur_dest_sgl->length;
+			destlen = PAGE_SIZE - cur_dest_sgl->length;
 
 			copylen = min(srclen, destlen);
 			memcpy((void *)dest, (void *)src, copylen);
 
 			total_copied += copylen;
-			bounce_sgl[j].length += copylen;
+			cur_dest_sgl->length += copylen;
 			srclen -= copylen;
 			src += copylen;
 
-			if (bounce_sgl[j].length == PAGE_SIZE) {
+			if (cur_dest_sgl->length == PAGE_SIZE) {
 				/* full..move to next entry */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 				bounce_addr = 0;
 				j++;
 			}
 
 			/* if we need to use another bounce buffer */
-			if (srclen && bounce_addr == 0)
-				bounce_addr = sg_kmap_atomic(bounce_sgl, j);
+			if (srclen && bounce_addr == 0) {
+				cur_dest_sgl = sg_next(cur_dest_sgl);
+				bounce_addr = (unsigned long)
+						kmap_atomic(
+						sg_page(cur_dest_sgl));
+			}
 
 		}
 
-		sg_kunmap_atomic(src_addr - orig_sgl[i].offset);
+		kunmap_atomic((void *)(src_addr - cur_src_sgl->offset));
+		cur_src_sgl = sg_next(cur_src_sgl);
 	}
 
 	if (bounce_addr)
-		sg_kunmap_atomic(bounce_addr);
+		kunmap_atomic((void *)bounce_addr);
 
 	local_irq_restore(flags);
 
@@ -1536,6 +1549,7 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	struct scatterlist *sgl;
 	unsigned int sg_count = 0;
 	struct vmscsi_request *vm_srb;
+	struct scatterlist *cur_sgl;
 
 	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
 		/*
@@ -1617,10 +1631,12 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 		}
 
 		cmd_request->data_buffer.offset = sgl[0].offset;
-
-		for (i = 0; i < sg_count; i++)
+		cur_sgl = sgl;
+		for (i = 0; i < sg_count; i++) {
 			cmd_request->data_buffer.pfn_array[i] =
-				page_to_pfn(sg_page((&sgl[i])));
+				page_to_pfn(sg_page((cur_sgl)));
+			cur_sgl = sg_next(cur_sgl);
+		}
 
 	} else if (scsi_sglist(scmnd)) {
 		cmd_request->data_buffer.offset =
Signature · 1 line
1.7.4.1
Reviewed-by on v1 6/7
#11 lore raw

RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

Quoted 13 lines

-----Original Message-----
From: devel [mailto:[email protected]] On Behalf Of K. Y. Srinivasan
Sent: Monday, March 23, 2015 2:07 PM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

Quoted 4 lines

The current code assumes that the scatterlists presented are not chained. Fix the code to not make this assumption.

Quoted 221 lines
---
 drivers/scsi/storvsc_drv.c |   98 +++++++++++++++++++++++++------------------
 1 files changed, 57 insertions(+), 41 deletions(-)
drivers/scsi/storvsc_drv.c index renamed from drivers/scsi/storvsc_drv.c +0 -0

88f5d79..a599677 100644

drivers/scsi/storvsc_drv.c +0 -11
@@ -626,19 +626,6 @@ cleanup:
 	return NULL;
 }
 
-/* Disgusting wrapper functions */
-static inline unsigned long sg_kmap_atomic(struct scatterlist *sgl, int idx) -{
-	void *addr = kmap_atomic(sg_page(sgl + idx));
-	return (unsigned long)addr;
-}
-
-static inline void sg_kunmap_atomic(unsigned long addr) -{
-	kunmap_atomic((void *)addr);
-}
-
-
 /* Assume the original sgl has enough room */  static unsigned int
 copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 					    struct scatterlist *bounce_sgl, @@ -

653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,

drivers/scsi/storvsc_drv.c +51 -25
 	unsigned long bounce_addr = 0;
 	unsigned long dest_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_dest_sgl;
+	struct scatterlist *cur_src_sgl;
 
 	local_irq_save(flags);
-
+	cur_dest_sgl = orig_sgl;
+	cur_src_sgl = bounce_sgl;
 	for (i = 0; i < orig_sgl_count; i++) {
-		dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		dest_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_dest_sgl)) +
+				cur_dest_sgl->offset;
 		dest = dest_addr;
 		destlen = orig_sgl[i].length;
+		destlen = cur_dest_sgl->length;
 
 		if (bounce_addr == 0)
-			bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+			bounce_addr = (unsigned long)kmap_atomic(
+							sg_page(cur_src_sgl));
 
 		while (destlen) {
-			src = bounce_addr + bounce_sgl[j].offset;
-			srclen = bounce_sgl[j].length - bounce_sgl[j].offset;
+			src = bounce_addr + cur_src_sgl->offset;
+			srclen = cur_src_sgl->length - cur_src_sgl->offset;
 
 			copylen = min(srclen, destlen);
 			memcpy((void *)dest, (void *)src, copylen);
 
 			total_copied += copylen;
-			bounce_sgl[j].offset += copylen;
+			cur_src_sgl->offset += copylen;
 			destlen -= copylen;
 			dest += copylen;
 
-			if (bounce_sgl[j].offset == bounce_sgl[j].length) {
+			if (cur_src_sgl->offset == cur_src_sgl->length) {
 				/* full */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 				j++;
 
 				/*
@@ -692,21 +686,27 @@ static unsigned int copy_from_bounce_buffer(struct
 scatterlist *orig_sgl,
 					/*
 					 * We are done; cleanup and return.
 					 */
-					sg_kunmap_atomic(dest_addr -
 orig_sgl[i].offset);
+					kunmap_atomic((void *)(dest_addr -
+						cur_dest_sgl->offset));
 					local_irq_restore(flags);
 					return total_copied;
 				}
 
 				/* if we need to use another bounce buffer */
-				if (destlen || i != orig_sgl_count - 1)
-					bounce_addr =
 sg_kmap_atomic(bounce_sgl,j);
+				if (destlen || i != orig_sgl_count - 1) {
+					cur_src_sgl = sg_next(cur_src_sgl);
+					bounce_addr = (unsigned long)
+							kmap_atomic(
+							sg_page(cur_src_sgl));
+				}
 			} else if (destlen == 0 && i == orig_sgl_count - 1) {
 				/* unmap the last bounce that is < PAGE_SIZE
 */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 			}
 		}
 
-		sg_kunmap_atomic(dest_addr - orig_sgl[i].offset);
+		kunmap_atomic((void *)(dest_addr - cur_dest_sgl->offset));
+		cur_dest_sgl = sg_next(cur_dest_sgl);
 	}
 
 	local_irq_restore(flags);
@@ -727,48 +727,61 @@ static unsigned int copy_to_bounce_buffer(struct
 scatterlist *orig_sgl,
 	unsigned long bounce_addr = 0;
 	unsigned long src_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_src_sgl;
+	struct scatterlist *cur_dest_sgl;
 
 	local_irq_save(flags);
 
+	cur_src_sgl = orig_sgl;
+	cur_dest_sgl = bounce_sgl;
+
 	for (i = 0; i < orig_sgl_count; i++) {
-		src_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		src_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_src_sgl)) +
+				cur_src_sgl->offset;
 		src = src_addr;
-		srclen = orig_sgl[i].length;
+		srclen = cur_src_sgl->length;
 
 		if (bounce_addr == 0)
-			bounce_addr = sg_kmap_atomic(bounce_sgl,j);
+			bounce_addr = (unsigned long)
+					kmap_atomic(sg_page(cur_dest_sgl));
 
 		while (srclen) {
 			/* assume bounce offset always == 0 */
-			dest = bounce_addr + bounce_sgl[j].length;
-			destlen = PAGE_SIZE - bounce_sgl[j].length;
+			dest = bounce_addr + cur_dest_sgl->length;
+			destlen = PAGE_SIZE - cur_dest_sgl->length;
 
 			copylen = min(srclen, destlen);
 			memcpy((void *)dest, (void *)src, copylen);
 
 			total_copied += copylen;
-			bounce_sgl[j].length += copylen;
+			cur_dest_sgl->length += copylen;
 			srclen -= copylen;
 			src += copylen;
 
-			if (bounce_sgl[j].length == PAGE_SIZE) {
+			if (cur_dest_sgl->length == PAGE_SIZE) {
 				/* full..move to next entry */
-				sg_kunmap_atomic(bounce_addr);
+				kunmap_atomic((void *)bounce_addr);
 				bounce_addr = 0;
 				j++;
 			}
 
 			/* if we need to use another bounce buffer */
-			if (srclen && bounce_addr == 0)
-				bounce_addr = sg_kmap_atomic(bounce_sgl,
 j);
+			if (srclen && bounce_addr == 0) {
+				cur_dest_sgl = sg_next(cur_dest_sgl);
+				bounce_addr = (unsigned long)
+						kmap_atomic(
+						sg_page(cur_dest_sgl));
+			}
 
 		}
 
-		sg_kunmap_atomic(src_addr - orig_sgl[i].offset);
+		kunmap_atomic((void *)(src_addr - cur_src_sgl->offset));
+		cur_src_sgl = sg_next(cur_src_sgl);
 	}
 
 	if (bounce_addr)
-		sg_kunmap_atomic(bounce_addr);
+		kunmap_atomic((void *)bounce_addr);
 
drivers/scsi/storvsc_drv.c +1 -0
 	local_irq_restore(flags);
@@ -1536,6 +1549,7 @@ static int storvsc_queuecommand(struct Scsi_Host
 *host, struct scsi_cmnd *scmnd)
 	struct scatterlist *sgl;
 	unsigned int sg_count = 0;
 	struct vmscsi_request *vm_srb;
+	struct scatterlist *cur_sgl;
 
 	if (vmstor_current_major <= VMSTOR_WIN8_MAJOR) {
drivers/scsi/storvsc_drv.c +5 -3
 		/*
@@ -1617,10 +1631,12 @@ static int storvsc_queuecommand(struct
 Scsi_Host *host, struct scsi_cmnd *scmnd)
 		}
 
 		cmd_request->data_buffer.offset = sgl[0].offset;
-
-		for (i = 0; i < sg_count; i++)
+		cur_sgl = sgl;
+		for (i = 0; i < sg_count; i++) {
 			cmd_request->data_buffer.pfn_array[i] =
-				page_to_pfn(sg_page((&sgl[i])));
+				page_to_pfn(sg_page((cur_sgl)));
+			cur_sgl = sg_next(cur_sgl);
+		}
 
 	} else if (scsi_sglist(scmnd)) {

cmd_request->data_buffer.offset =
--
1.7.4.1

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

#12 lore raw

Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

In reply to K. Y. Srinivasan
Quoted 9 lines · K. Y. Srinivasan
drivers/scsi/storvsc_drv.c +2 -1
@@ -653,32 +640,39 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
 	unsigned long bounce_addr = 0;
 	unsigned long dest_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_dest_sgl;
+	struct scatterlist *cur_src_sgl;
 
 	local_irq_save(flags);
-

drivers/scsi/storvsc_drv.c static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl, · line 655

Quoted 10 lines
drivers/scsi/storvsc_drv.c +6 -1
+	cur_dest_sgl = orig_sgl;
+	cur_src_sgl = bounce_sgl;
 	for (i = 0; i < orig_sgl_count; i++) {
-		dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		dest_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_dest_sgl)) +
+				cur_dest_sgl->offset;
 		dest = dest_addr;
 		destlen = orig_sgl[i].length;
+		destlen = cur_dest_sgl->length;

Double assignment.

Olaf

#13 lore raw

RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

Quoted 23 lines

-----Original Message-----
From: Olaf Hering [mailto:[email protected]]
Sent: Tuesday, March 24, 2015 1:56 AM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained

Review static unsigned int

Quoted 1 line · K. Y. Srinivasan
(quoted diff) +0 -0
@@ -653,32 +640,39 @@ static unsigned int

copy_from_bounce_buffer(struct scatterlist *orig_sgl,

Quoted 6 lines
drivers/scsi/storvsc_drv.c +2 -0
 	unsigned long bounce_addr = 0;
 	unsigned long dest_addr = 0;
 	unsigned long flags;
+	struct scatterlist *cur_dest_sgl;
+	struct scatterlist *cur_src_sgl;
 
Quoted 14 lines

drivers/scsi/storvsc_drv.c static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl, · line 655

Quoted 12 lines
drivers/scsi/storvsc_drv.c +6 -2
 	local_irq_save(flags);
-
+	cur_dest_sgl = orig_sgl;
+	cur_src_sgl = bounce_sgl;
 	for (i = 0; i < orig_sgl_count; i++) {
-		dest_addr = sg_kmap_atomic(orig_sgl,i) + orig_sgl[i].offset;
+		dest_addr = (unsigned long)
+				kmap_atomic(sg_page(cur_dest_sgl)) +
+				cur_dest_sgl->offset;
 		dest = dest_addr;
 		destlen = orig_sgl[i].length;
+		destlen = cur_dest_sgl->length;

Double assignment.

Thanks Olaf; I will fix this and resubmit.

K. Y

Quoted 2 lines

Olaf