* [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: 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 <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++-----------
1 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 27fe850..5a12897 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
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 <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++-----------
1 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 27fe850..5a12897 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size
2015-03-23 21:06 ` K. Y. Srinivasan
(?)
@ 2015-03-24 0:23 ` Venkatesh Srinivas
2015-03-24 1:49 ` KY Srinivasan
-1 siblings, 1 reply; 22+ messages in thread
From: Venkatesh Srinivas @ 2015-03-24 0:23 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, Linux Kernel Developers List, devel, ohering,
James E.J. Bottomley, Christoph Hellwig, linux-scsi, apw,
vkuznets, jasowang
On Mon, Mar 23, 2015 at 2:06 PM, K. Y. Srinivasan <kys@microsoft.com> wrote:
> 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 <kys@microsoft.com>
> Reviewed-by: Long Li <longli@microsoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++-----------
> 1 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 27fe850..5a12897 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -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);
> +
If num_online_cpus() returned 1 - 3, can_queue will be set to 1 I
think. Is that desired?
> 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);
> }
-- vs;
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size
2015-03-24 0:23 ` Venkatesh Srinivas
@ 2015-03-24 1:49 ` KY Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: KY Srinivasan @ 2015-03-24 1:49 UTC (permalink / raw)
To: Venkatesh Srinivas
Cc: gregkh, Linux Kernel Developers List, devel, ohering,
James E.J. Bottomley, Christoph Hellwig, linux-scsi, apw,
vkuznets, jasowang
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5057 bytes --]
> -----Original Message-----
> From: Venkatesh Srinivas [mailto:venkateshs@google.com]
> Sent: Monday, March 23, 2015 5:23 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; Linux Kernel Developers List;
> devel@linuxdriverproject.org; ohering@suse.com; James E.J. Bottomley;
> Christoph Hellwig; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based
> on the ringbuffer size
>
> On Mon, Mar 23, 2015 at 2:06 PM, K. Y. Srinivasan <kys@microsoft.com>
> wrote:
> > 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 <kys@microsoft.com>
> > Reviewed-by: Long Li <longli@microsoft.com>
> > ---
> > drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++-----------
> > 1 files changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 27fe850..5a12897 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -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);
> > +
>
> 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
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based on the ringbuffer size
@ 2015-03-24 1:49 ` KY Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: KY Srinivasan @ 2015-03-24 1:49 UTC (permalink / raw)
To: Venkatesh Srinivas
Cc: gregkh, Linux Kernel Developers List, devel, ohering,
James E.J. Bottomley, Christoph Hellwig, linux-scsi, apw,
vkuznets, jasowang
> -----Original Message-----
> From: Venkatesh Srinivas [mailto:venkateshs@google.com]
> Sent: Monday, March 23, 2015 5:23 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; Linux Kernel Developers List;
> devel@linuxdriverproject.org; ohering@suse.com; James E.J. Bottomley;
> Christoph Hellwig; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH RESEND 2/7] scsi: storvsc: Size the queue depth based
> on the ringbuffer size
>
> On Mon, Mar 23, 2015 at 2:06 PM, K. Y. Srinivasan <kys@microsoft.com>
> wrote:
> > 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 <kys@microsoft.com>
> > Reviewed-by: Long Li <longli@microsoft.com>
> > ---
> > drivers/scsi/storvsc_drv.c | 27 ++++++++++++++++-----------
> > 1 files changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 27fe850..5a12897 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -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);
> > +
>
> 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH RESEND 3/7] scsi: storvsc: Always send on the selected outgoing channel
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: 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.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5a12897..be8f12e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH RESEND 3/7] scsi: storvsc: Always send on the selected outgoing channel
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
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.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5a12897..be8f12e 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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,
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH RESEND 4/7] scsi: storvsc: Retrieve information about the capability of the target
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: 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.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index be8f12e..5c13eec 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH RESEND 4/7] scsi: storvsc: Retrieve information about the capability of the target
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
The storage protocol informs the guest of the I/O capabilities of the storage
stack. Retrieve this information and use it in the guest.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index be8f12e..5c13eec 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan, stable
We may exit this function without properly freeing up the maapings
we may have acquired. Fix the bug.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
drivers/scsi/storvsc_drv.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5c13eec..88f5d79 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan, stable
We may exit this function without properly freeing up the maapings
we may have acquired. Fix the bug.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Cc: <stable@vger.kernel.org>
---
drivers/scsi/storvsc_drv.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5c13eec..88f5d79 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
2015-03-23 21:06 ` K. Y. Srinivasan
(?)
@ 2015-03-23 20:52 ` Long Li
-1 siblings, 0 replies; 22+ messages in thread
From: Long Li @ 2015-03-23 20:52 UTC (permalink / raw)
To: KY Srinivasan, gregkh, linux-kernel, devel, ohering, jbottomley,
hch, linux-scsi, apw, vkuznets, jasowang
Cc: stable
> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Monday, March 23, 2015 2:07 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Cc: stable@vger.kernel.org
> Subject: [PATCH 5/7] scsi: storvsc: Fix a bug in copy_from_bounce_buffer()
>
> We may exit this function without properly freeing up the maapings we may
> have acquired. Fix the bug.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/scsi/storvsc_drv.c | 15 ++++++++-------
> 1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 5c13eec..88f5d79 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -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
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan
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 <kys@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 98 +++++++++++++++++++++++++------------------
1 files changed, 57 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 88f5d79..a599677 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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 =
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
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 <kys@microsoft.com>
---
drivers/scsi/storvsc_drv.c | 98 +++++++++++++++++++++++++------------------
1 files changed, 57 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 88f5d79..a599677 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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 =
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
2015-03-23 21:06 ` K. Y. Srinivasan
(?)
@ 2015-03-23 20:52 ` Long Li
-1 siblings, 0 replies; 22+ messages in thread
From: Long Li @ 2015-03-23 20:52 UTC (permalink / raw)
To: KY Srinivasan, gregkh, linux-kernel, devel, ohering, jbottomley,
hch, linux-scsi, apw, vkuznets, jasowang
> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of K. Y. Srinivasan
> Sent: Monday, March 23, 2015 2:07 PM
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; ohering@suse.com; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not
> chained
>
> 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 <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
> ---
> drivers/scsi/storvsc_drv.c | 98 +++++++++++++++++++++++++------------------
> 1 files changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> 88f5d79..a599677 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -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 =
> --
> 1.7.4.1
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
2015-03-23 21:06 ` K. Y. Srinivasan
(?)
(?)
@ 2015-03-24 8:56 ` Olaf Hering
2015-03-24 16:29 ` KY Srinivasan
-1 siblings, 1 reply; 22+ messages in thread
From: Olaf Hering @ 2015-03-24 8:56 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, jbottomley, hch, linux-scsi, apw,
vkuznets, jasowang
On Mon, Mar 23, K. Y. Srinivasan wrote:
> @@ -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;
Double assignment.
Olaf
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
2015-03-24 8:56 ` Olaf Hering
@ 2015-03-24 16:29 ` KY Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: KY Srinivasan @ 2015-03-24 16:29 UTC (permalink / raw)
To: Olaf Hering
Cc: gregkh, linux-kernel, devel, jbottomley, hch, linux-scsi, apw,
vkuznets, jasowang
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1462 bytes --]
> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Tuesday, March 24, 2015 1:56 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not
> chained
>
> On Mon, Mar 23, K. Y. Srinivasan wrote:
>
> > @@ -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;
>
> Double assignment.
Thanks Olaf; I will fix this and resubmit.
K. Y
>
> Olaf
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not chained
@ 2015-03-24 16:29 ` KY Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: KY Srinivasan @ 2015-03-24 16:29 UTC (permalink / raw)
To: Olaf Hering
Cc: linux-scsi, gregkh, jasowang, linux-kernel, jbottomley, hch, apw, devel
> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Tuesday, March 24, 2015 1:56 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; jbottomley@parallels.com;
> hch@infradead.org; linux-scsi@vger.kernel.org; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: Re: [PATCH 6/7] scsi: storvsc: Don't assume that the scatterlist is not
> chained
>
> On Mon, Mar 23, K. Y. Srinivasan wrote:
>
> > @@ -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;
>
> Double assignment.
Thanks Olaf; I will fix this and resubmit.
K. Y
>
> Olaf
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V2 7/7] scsi: storvsc: Set the tablesize based on the information given by the host
2015-03-23 21:06 ` [PATCH RESEND 1/7] scsi: storvsc: Increase the ring buffer size K. Y. Srinivasan
@ 2015-03-23 21:06 ` K. Y. Srinivasan
2015-03-23 21:06 ` K. Y. Srinivasan
` (4 subsequent siblings)
5 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Cc: K. Y. Srinivasan
Set the tablesize based on the information given by the host.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
v2: Rebase the code and enable clustering
drivers/scsi/storvsc_drv.c | 89 +++++++++++++++++++++++++++++++------------
1 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index a599677..71871f2 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH V2 7/7] scsi: storvsc: Set the tablesize based on the information given by the host
@ 2015-03-23 21:06 ` K. Y. Srinivasan
0 siblings, 0 replies; 22+ messages in thread
From: K. Y. Srinivasan @ 2015-03-23 21:06 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, ohering, jbottomley, hch,
linux-scsi, apw, vkuznets, jasowang
Set the tablesize based on the information given by the host.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Long Li <longli@microsoft.com>
---
v2: Rebase the code and enable clustering
drivers/scsi/storvsc_drv.c | 89 +++++++++++++++++++++++++++++++------------
1 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index a599677..71871f2 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread