scsi: storvsc: Miscellaneous enhancements and fixes
Subsystems (3)
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
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
@@ -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
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
@@ -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 200static 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
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.
Signed-off-by K. Y. Srinivasan <[email protected]>Reviewed-by Long Li <[email protected]>--- drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++----------- 1 files changed, 16 insertions(+), 11 deletions(-)drivers/scsi/storvsc_drv.c +12
-3@@ -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 200static 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
-1host =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;
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 sizeQuoted 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.
Signed-off-by K. Y. Srinivasan <[email protected]>Reviewed-by Long Li <[email protected]>--- drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++----------- 1 files changed, 16 insertions(+), 11 deletions(-)drivers/scsi/storvsc_drv.c +0
-0drivers/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 tosubchannels");
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 200static 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
-0max_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
-3break ; } -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_queuewill 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
PATCH RESEND 3/7 scsi: storvsc: Always send on the selected outgoing channel
In reply to K. Y. SrinivasanThe 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
@@ -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
PATCH RESEND 4/7 scsi: storvsc: Retrieve information about the capability of the target
In reply to K. Y. SrinivasanThe 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
@@ -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
PATCH 5/7 scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
In reply to K. Y. SrinivasanWe 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
@@ -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
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 incopy_from_bounce_buffer()
Quoted 4 lines
We may exit this function without properly freeing up the maapings we may have acquired. Fix the bug.
Signed-off-by K. Y. Srinivasan <[email protected]>
Quoted 50 lines
Cc <[email protected]>--- 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
-05c13eec..88f5d79 100644
drivers/scsi/storvsc_drv.c +8
-8@@ -754,21 +754,22 @@ static unsigned int copy_to_bounce_buffer(structscatterlist *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
PATCH v2 7/7 scsi: storvsc: Set the tablesize based on the information given by the host
In reply to K. Y. SrinivasanSet 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
@@ -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
PATCH 6/7 scsi: storvsc: Don't assume that the scatterlist is not chained
In reply to K. Y. SrinivasanThe 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
@@ -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
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.
Signed-off-by K. Y. Srinivasan <[email protected]>
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
-088f5d79..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
-25unsigned 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(structscatterlist *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
-0local_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
Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
In reply to K. Y. SrinivasanQuoted 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
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 chainedReview static unsigned int
Quoted 1 line · K. Y. Srinivasan
(quoted diff) +0
-0@@ -653,32 +640,39 @@ static unsigned intcopy_from_bounce_buffer(struct scatterlist
*orig_sgl,Quoted 6 lines
drivers/scsi/storvsc_drv.c +2
-0unsigned 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
-2local_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