RE: [PATCH 0/6] Drivers: hv/scsi: Implement multi-channel support

2013-05-16 Thread KY Srinivasan


 -Original Message-
 From: Greg KH [mailto:gre...@linuxfoundation.org]
 Sent: Thursday, May 16, 2013 12:00 AM
 To: KY Srinivasan
 Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
 oher...@suse.com; jbottom...@parallels.com; h...@infradead.org; linux-
 s...@vger.kernel.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH 0/6] Drivers: hv/scsi: Implement multi-channel support
 
 On Wed, May 15, 2013 at 03:02:00PM -0700, K. Y. Srinivasan wrote:
  This patch-set implements multi-channel support for Hyper-V devices. Also
  support for synthetic Fiber Channel device is included. The first two
  patches in the series are the foundational pieces for the remaining patches.
 
  James, once Greg accepts the first two patches, you can consider the scsi
  patches.
 
 I have no objection for these first two patches to go through James's
 tree, I'll go add my ack to them.  That should make it easier for
 everyone involved.

Thanks Greg.

Regards,

K. Y
 


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support

2013-05-16 Thread KY Srinivasan


 -Original Message-
 From: Greg KH [mailto:gre...@linuxfoundation.org]
 Sent: Thursday, May 16, 2013 12:02 AM
 To: KY Srinivasan
 Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
 oher...@suse.com; jbottom...@parallels.com; h...@infradead.org; linux-
 s...@vger.kernel.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support
 
 On Wed, May 15, 2013 at 03:02:29PM -0700, K. Y. Srinivasan wrote:
  +/*
  + * Retrieve the (sub) channel on which to send an outgoing request.
  + * When a primary channel has multiple sub-channels, we choose a
  + * channel whose VCPU binding is closest to the VCPU on which
  + * this call is being made.
  + */
  +struct vmbus_channel *get_outgoing_channel(struct vmbus_channel
 *primary)
 
 That's a _very_ vague global symbol name you are adding to the kernel.
 Same goes for the other functions you are adding here, please fix that,
 and make them have the vmbus_ prefix, like everything else in this
 patch.


Will do.

K. Y


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V1 0/7] Drivers: hv/scsi: Implement multi-channel support

2013-05-16 Thread K. Y. Srinivasan
This patch-set implements multi-channel support for Hyper-V devices. Also
support for synthetic Fiber Channel device is included. The first two
patches in the series are the foundational pieces for many of the remaining
patches.

James, once Greg accepts the first two patches, you can consider the scsi
patches.

In this version, based on Greg's comments, I have fixed the names of the
exported symbols from the vmbus driver.

K. Y. Srinivasan (7):
  Drivers: hv: vmbus: Implement multi-channel support
  Drivers: hv: Add the GUID fot synthetic fiber channel device
  Drivers: scsi: storvsc: Make the scsi timeout a module parameter
  Drivers: scsi: storvsc: Update the storage protocol to win8 level
  Drivers: scsi: storvsc: Implement multi-channel support
  Drivers: scsi: storvsc: Support FC devices
  Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

 drivers/hv/channel.c   |   41 +-
 drivers/hv/channel_mgmt.c  |  116 ++-
 drivers/scsi/storvsc_drv.c |  342 +++-
 include/linux/hyperv.h |   70 +
 4 files changed, 522 insertions(+), 47 deletions(-)

-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V1 2/7] Drivers: hv: Add the GUID fot synthetic fiber channel device

2013-05-16 Thread K. Y. Srinivasan
In preparation for supporting synthetic Fiber Channel device, add the GUID for
this service.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
---
 include/linux/hyperv.h |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 405e05a..fae8bac 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1389,6 +1389,16 @@ void vmbus_driver_unregister(struct hv_driver 
*hv_driver);
}
 
 /*
+ * Synthetic FC GUID
+ * {2f9bcc4a-0069-4af3-b76b-6fd0be528cda}
+ */
+#define HV_SYNTHFC_GUID \
+   .guid = { \
+   0x4A, 0xCC, 0x9B, 0x2F, 0x69, 0x00, 0xF3, 0x4A, \
+   0xB7, 0x6B, 0x6F, 0xD0, 0xBE, 0x52, 0x8C, 0xDA \
+   }
+
+/*
  * Common header for Hyper-V ICs
  */
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V1 1/7] Drivers: hv: vmbus: Implement multi-channel support

2013-05-16 Thread K. Y. Srinivasan
Starting with Win8, the host supports multiple sub-channels for a given
device. As in the past, the initial channel offer specifies the device and
is associated with both the type and the instance GUIDs. For performance
critical devices, the host may support multiple sub-channels. The sub-channels
share the same type and instance GUID as the primary channel. The number of
sub-channels offerrred to the guest depends on the number of virtual CPUs
assigned to the guest. The guest can request the creation of these sub-channels
and once created and opened, the guest can distribute the traffic across all
the channels (the primary and the sub-channels). A request sent on a sub-channel
will have the response delivered on the same sub-channel.

At channel (sub-channel) creation we bind the channel interrupt to a CPU and
with this sub-channel support we will be able to spread the interrupt load
of a given device across all available CPUs.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
---
 drivers/hv/channel.c  |   41 ++--
 drivers/hv/channel_mgmt.c |  116 +++--
 include/linux/hyperv.h|   62 +++-
 3 files changed, 210 insertions(+), 9 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 0b122f8..c219e9f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -216,6 +216,9 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
send_ringbuffer_size,
list_del(open_info-msglistentry);
spin_unlock_irqrestore(vmbus_connection.channelmsg_lock, flags);
 
+   if (err == 0)
+   newchannel-state = CHANNEL_OPENED_STATE;
+
kfree(open_info);
return err;
 
@@ -500,15 +503,14 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, 
u32 gpadl_handle)
 }
 EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
 
-/*
- * vmbus_close - Close the specified channel
- */
-void vmbus_close(struct vmbus_channel *channel)
+static void vmbus_close_internal(struct vmbus_channel *channel)
 {
struct vmbus_channel_close_channel *msg;
int ret;
unsigned long flags;
 
+   channel-state = CHANNEL_OPEN_STATE;
+   channel-sc_creation_callback = NULL;
/* Stop callback and cancel the timer asap */
spin_lock_irqsave(channel-inbound_lock, flags);
channel-onchannel_callback = NULL;
@@ -538,6 +540,37 @@ void vmbus_close(struct vmbus_channel *channel)
 
 
 }
+
+/*
+ * vmbus_close - Close the specified channel
+ */
+void vmbus_close(struct vmbus_channel *channel)
+{
+   struct list_head *cur, *tmp;
+   struct vmbus_channel *cur_channel;
+
+   if (channel-primary_channel != NULL) {
+   /*
+* We will only close sub-channels when
+* the primary is closed.
+*/
+   return;
+   }
+   /*
+* Close all the sub-channels first and then close the
+* primary channel.
+*/
+   list_for_each_safe(cur, tmp, channel-sc_list) {
+   cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
+   if (cur_channel-state != CHANNEL_OPENED_STATE)
+   continue;
+   vmbus_close_internal(cur_channel);
+   }
+   /*
+* Now close the primary.
+*/
+   vmbus_close_internal(channel);
+}
 EXPORT_SYMBOL_GPL(vmbus_close);
 
 /**
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 21ef689..8a8b624 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -115,6 +115,9 @@ static struct vmbus_channel *alloc_channel(void)
return NULL;
 
spin_lock_init(channel-inbound_lock);
+   spin_lock_init(channel-sc_lock);
+
+   INIT_LIST_HEAD(channel-sc_list);
 
channel-controlwq = create_workqueue(hv_vmbus_ctl);
if (!channel-controlwq) {
@@ -166,6 +169,7 @@ static void vmbus_process_rescind_offer(struct work_struct 
*work)
 struct vmbus_channel,
 work);
unsigned long flags;
+   struct vmbus_channel *primary_channel;
struct vmbus_channel_relid_released msg;
 
vmbus_device_unregister(channel-device_obj);
@@ -174,9 +178,16 @@ static void vmbus_process_rescind_offer(struct work_struct 
*work)
msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
vmbus_post_msg(msg, sizeof(struct vmbus_channel_relid_released));
 
-   spin_lock_irqsave(vmbus_connection.channel_lock, flags);
-   list_del(channel-listentry);
-   spin_unlock_irqrestore(vmbus_connection.channel_lock, flags);
+   if (channel-primary_channel == NULL) {
+   spin_lock_irqsave(vmbus_connection.channel_lock, flags);
+   list_del(channel-listentry);
+   spin_unlock_irqrestore(vmbus_connection.channel_lock, flags);
+   } else 

[PATCH V1 3/7] Drivers: scsi: storvsc: Make the scsi timeout a module parameter

2013-05-16 Thread K. Y. Srinivasan
The standard scsi timeout is not appropriate in some of the environments where
Hyper-V is deployed. Set this timeout appropriately for all devices managed
by this driver. Further make this a module parameter.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
---
 drivers/scsi/storvsc_drv.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 16a3a0c..8d29a95 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -221,6 +221,13 @@ static int storvsc_ringbuffer_size = (20 * PAGE_SIZE);
 module_param(storvsc_ringbuffer_size, int, S_IRUGO);
 MODULE_PARM_DESC(storvsc_ringbuffer_size, Ring buffer size (bytes));
 
+/*
+ * Timeout in seconds for all devices managed by this driver.
+ */
+static int storvsc_timeout = 180;
+module_param(storvsc_timeout, uint, (S_IRUGO | S_IWUSR));
+MODULE_PARM_DESC(storvsc_timeout, Device timeout (seconds));
+
 #define STORVSC_MAX_IO_REQUESTS128
 
 /*
@@ -1204,6 +1211,8 @@ static int storvsc_device_configure(struct scsi_device 
*sdevice)
 
blk_queue_bounce_limit(sdevice-request_queue, BLK_BOUNCE_ANY);
 
+   blk_queue_rq_timeout(sdevice-request_queue, (storvsc_timeout * HZ));
+
sdevice-no_write_same = 1;
 
return 0;
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V1 5/7] Drivers: scsi: storvsc: Implement multi-channel support

2013-05-16 Thread K. Y. Srinivasan
Implement multi-channel support for the storage devices.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
---
 drivers/scsi/storvsc_drv.c |  124 ++--
 1 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index a3cf85c..6bc4e3b 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -330,6 +330,8 @@ MODULE_PARM_DESC(storvsc_timeout, Device timeout 
(seconds));
 
 #define STORVSC_MAX_IO_REQUESTS128
 
+static void storvsc_on_channel_callback(void *context);
+
 /*
  * In Hyper-V, each port/path/target maps to 1 scsi host adapter.  In
  * reality, the path/target is not used (ie always set to 0) so our
@@ -757,12 +759,99 @@ static unsigned int copy_to_bounce_buffer(struct 
scatterlist *orig_sgl,
return total_copied;
 }
 
+static void handle_sc_creation(struct vmbus_channel *new_sc)
+{
+   struct hv_device *device = new_sc-primary_channel-device_obj;
+   struct storvsc_device *stor_device;
+   struct vmstorage_channel_properties props;
+
+   stor_device = get_out_stor_device(device);
+   if (!stor_device)
+   return;
+
+
+   memset(props, 0, sizeof(struct vmstorage_channel_properties));
+
+   vmbus_open(new_sc,
+  storvsc_ringbuffer_size,
+  storvsc_ringbuffer_size,
+  (void *)props,
+  sizeof(struct vmstorage_channel_properties),
+  storvsc_on_channel_callback, new_sc);
+}
+
+static void  handle_multichannel_storage(struct hv_device *device, int 
max_chns)
+{
+   struct storvsc_device *stor_device;
+   int num_cpus = num_online_cpus();
+   int num_sc;
+   struct storvsc_cmd_request *request;
+   struct vstor_packet *vstor_packet;
+   int ret, t;
+
+   num_sc = ((max_chns  num_cpus) ? num_cpus : max_chns);
+   stor_device = get_out_stor_device(device);
+   if (!stor_device)
+   return;
+
+   request = stor_device-init_request;
+   vstor_packet = request-vstor_packet;
+
+   /*
+* Establish a handler for dealing with subchannels.
+*/
+   vmbus_set_sc_create_callback(device-channel, handle_sc_creation);
+
+   /*
+* Check to see if sub-channels have already been created. This
+* can happen when this driver is re-loaded after unloading.
+*/
+
+   if (vmbus_are_subchannels_present(device-channel))
+   return;
+
+   /*
+* Request the host to create sub-channels.
+*/
+   memset(request, 0, sizeof(struct storvsc_cmd_request));
+   init_completion(request-wait_event);
+   vstor_packet-operation = VSTOR_OPERATION_CREATE_SUB_CHANNELS;
+   vstor_packet-flags = REQUEST_COMPLETION_FLAG;
+   vstor_packet-sub_channel_count = num_sc;
+
+   ret = vmbus_sendpacket(device-channel, vstor_packet,
+  (sizeof(struct vstor_packet) -
+  vmscsi_size_delta),
+  (unsigned long)request,
+  VM_PKT_DATA_INBAND,
+  VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+
+   if (ret != 0)
+   return;
+
+   t = wait_for_completion_timeout(request-wait_event, 10*HZ);
+   if (t == 0)
+   return;
+
+   if (vstor_packet-operation != VSTOR_OPERATION_COMPLETE_IO ||
+   vstor_packet-status != 0)
+   return;
+
+   /*
+* Now that we created the sub-channels, invoke the check; this
+* will trigger the callback.
+*/
+   vmbus_are_subchannels_present(device-channel);
+}
+
 static int storvsc_channel_init(struct hv_device *device)
 {
struct storvsc_device *stor_device;
struct storvsc_cmd_request *request;
struct vstor_packet *vstor_packet;
int ret, t;
+   int max_chns;
+   bool process_sub_channels = false;
 
stor_device = get_out_stor_device(device);
if (!stor_device)
@@ -857,6 +946,19 @@ static int storvsc_channel_init(struct hv_device *device)
vstor_packet-status != 0)
goto cleanup;
 
+   /*
+* Check to see if multi-channel support is there.
+* Hosts that implement protocol version of 5.1 and above
+* support multi-channel.
+*/
+   max_chns = vstor_packet-storage_channel_properties.max_channel_cnt;
+   if ((vmbus_proto_version != VERSION_WIN7) 
+  (vmbus_proto_version != VERSION_WS2008))  {
+   if (vstor_packet-storage_channel_properties.flags 
+   STORAGE_CHANNEL_SUPPORTS_MULTI_CHANNEL)
+   process_sub_channels = true;
+   }
+
memset(vstor_packet, 0, sizeof(struct vstor_packet));
vstor_packet-operation = 

[PATCH V1 6/7] Drivers: scsi: storvsc: Support FC devices

2013-05-16 Thread K. Y. Srinivasan
Add support for synthetic Fiber Channel devices.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@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 6bc4e3b..124cf0b 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1698,6 +1698,7 @@ static struct scsi_host_template scsi_driver = {
 enum {
SCSI_GUID,
IDE_GUID,
+   SFC_GUID,
 };
 
 static const struct hv_vmbus_device_id id_table[] = {
@@ -1709,6 +1710,11 @@ static const struct hv_vmbus_device_id id_table[] = {
{ HV_IDE_GUID,
  .driver_data = IDE_GUID
},
+   /* Fiber Channel GUID */
+   {
+ HV_SYNTHFC_GUID,
+ .driver_data = SFC_GUID
+   },
{ },
 };
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread K. Y. Srinivasan
Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The current
ringbuffer size can support this higher value.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@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 124cf0b..e2ab7cd 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -328,7 +328,7 @@ static int storvsc_timeout = 180;
 module_param(storvsc_timeout, uint, (S_IRUGO | S_IWUSR));
 MODULE_PARM_DESC(storvsc_timeout, Device timeout (seconds));
 
-#define STORVSC_MAX_IO_REQUESTS128
+#define STORVSC_MAX_IO_REQUESTS200
 
 static void storvsc_on_channel_callback(void *context);
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread Dan Carpenter
On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote:
 Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The current
 ringbuffer size can support this higher value.
 

The ringbuffer size is a module parameter so it's odd to talk about
the current size.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread KY Srinivasan


 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, May 16, 2013 8:02 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com;
 h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
 jasow...@redhat.com
 Subject: Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of
 STORVSC_MAX_IO_REQUESTS
 
 On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote:
  Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The
 current
  ringbuffer size can support this higher value.
 
 
 The ringbuffer size is a module parameter so it's odd to talk about
 the current size.

While the ringbuffer size is a module parameter; there is a default value. The 
current size refers to the default. 
Your comment applies to the current value (of 128) as well in that it is 
possible for somebody to load this
driver  with a ringbuffer size that could not support  the value of 128. If 
this  is the case, we fail the load.
This safety check continues to exist. 

Regards,

K. Y
 
 regards,
 dan carpenter
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread Dan Carpenter
On Thu, May 16, 2013 at 01:37:41PM +, KY Srinivasan wrote:
 
 
  -Original Message-
  From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
  Sent: Thursday, May 16, 2013 8:02 AM
  To: KY Srinivasan
  Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
  de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com;
  h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
  jasow...@redhat.com
  Subject: Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of
  STORVSC_MAX_IO_REQUESTS
  
  On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote:
   Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The
  current
   ringbuffer size can support this higher value.
  
  
  The ringbuffer size is a module parameter so it's odd to talk about
  the current size.
 
 While the ringbuffer size is a module parameter; there is a default value. 
 The current size refers to the default. 
 Your comment applies to the current value (of 128) as well in that it is 
 possible for somebody to load this
 driver  with a ringbuffer size that could not support  the value of 128. If 
 this  is the case, we fail the load.
 This safety check continues to exist. 

The issue is there in the original code, true.

Would the right fix be to add some sanity checks in module_init()?

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread KY Srinivasan


 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, May 16, 2013 9:55 AM
 To: KY Srinivasan
 Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; oher...@suse.com; jbottom...@parallels.com;
 h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
 jasow...@redhat.com
 Subject: Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of
 STORVSC_MAX_IO_REQUESTS
 
 On Thu, May 16, 2013 at 01:37:41PM +, KY Srinivasan wrote:
 
 
   -Original Message-
   From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
   Sent: Thursday, May 16, 2013 8:02 AM
   To: KY Srinivasan
   Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
   de...@linuxdriverproject.org; oher...@suse.com;
 jbottom...@parallels.com;
   h...@infradead.org; linux-scsi@vger.kernel.org; a...@canonical.com;
   jasow...@redhat.com
   Subject: Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of
   STORVSC_MAX_IO_REQUESTS
  
   On Thu, May 16, 2013 at 05:21:19AM -0700, K. Y. Srinivasan wrote:
Increase the value of STORVSC_MAX_IO_REQUESTS to 200 requests. The
   current
ringbuffer size can support this higher value.
   
  
   The ringbuffer size is a module parameter so it's odd to talk about
   the current size.
 
  While the ringbuffer size is a module parameter; there is a default value. 
  The
 current size refers to the default.
  Your comment applies to the current value (of 128) as well in that it is 
  possible
 for somebody to load this
  driver  with a ringbuffer size that could not support  the value of 128. If 
  this  is
 the case, we fail the load.
  This safety check continues to exist.
 
 The issue is there in the original code, true.
 
 Would the right fix be to add some sanity checks in module_init()?

The check is already there (as I noted above). Look at the function:
storvsc_drv_init(). If the ring size is picked incorrectly, the load is failed.

Regards,

K. Y
 
 regards,
 dan carpenter
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V1 1/7] Drivers: hv: vmbus: Implement multi-channel support

2013-05-16 Thread Greg KH
On Thu, May 16, 2013 at 05:21:13AM -0700, K. Y. Srinivasan wrote:
 Starting with Win8, the host supports multiple sub-channels for a given
 device. As in the past, the initial channel offer specifies the device and
 is associated with both the type and the instance GUIDs. For performance
 critical devices, the host may support multiple sub-channels. The sub-channels
 share the same type and instance GUID as the primary channel. The number of
 sub-channels offerrred to the guest depends on the number of virtual CPUs
 assigned to the guest. The guest can request the creation of these 
 sub-channels
 and once created and opened, the guest can distribute the traffic across all
 the channels (the primary and the sub-channels). A request sent on a 
 sub-channel
 will have the response delivered on the same sub-channel.
 
 At channel (sub-channel) creation we bind the channel interrupt to a CPU and
 with this sub-channel support we will be able to spread the interrupt load
 of a given device across all available CPUs.
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 Reviewed-by: Haiyang Zhang haiya...@microsoft.com

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V1 2/7] Drivers: hv: Add the GUID fot synthetic fiber channel device

2013-05-16 Thread Greg KH
On Thu, May 16, 2013 at 05:21:14AM -0700, K. Y. Srinivasan wrote:
 In preparation for supporting synthetic Fiber Channel device, add the GUID for
 this service.
 
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 Reviewed-by: Haiyang Zhang haiya...@microsoft.com

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V1 7/7] Drivers: scsi: storvsc: Increase the value of STORVSC_MAX_IO_REQUESTS

2013-05-16 Thread Dan Carpenter
On Thu, May 16, 2013 at 02:01:12PM +, KY Srinivasan wrote:
  Would the right fix be to add some sanity checks in module_init()?
 
 The check is already there (as I noted above). Look at the function:
 storvsc_drv_init(). If the ring size is picked incorrectly, the load is 
 failed.
 

Ah.  I see that now.  My mistake.

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] scsi: Port buslogic driver to 64 bits

2013-05-16 Thread Khalid Aziz

This patchset ports buslogic driver to 64-bit.

Current buslogic driver is composed of two components - SCCB manager
which communicates with adapter to execute SCSI commands (contained in
FlashPoint.c), and Linux driver part that interfaces with rest of the
kernel (contained in BusLogic.c). SCCB manager code came from the
Mylex SDK. SCCB manager code is used by flashpoint adapters only.
Multimaster adapters do not need SCCB manager since the SCSI state
machine is implemented in firmware on these adapters. If the filenames
alone did not give it away already, buslogic driver code is full of
CamelCase, besides being full of very ling lines, and is just very
difficult to read and understand. So the first step was to clean up
the existing code. First patch in the set does just that. Second patch
includes necessary code modifications to allow the driver to build and
run on 64-bit kernel. Since SCCB manager code came from Mylex SDK, I
have tried to touch it only when necessary which includes not fixing
all CamelCase issues in FlashPoint.c. Many lines over 80 characters
remain in BusLogic.c. These fall into two categories generally -
(1) it prints a message and I didn't want to touch driver messages
in case there are scripts out there that parse driver messages,
(2) code is indented deeply and is hard to keep it under 80 characters.
Such code could use refactoring at some point.

I have tested this patch with a flashpoint adapter on 64-bit and 
32-bit kernels with fio running random read/write test while verifying
data. I also measured performance for current buslogic driver and 
buslogic driver with these patches with 32-bit and 64-bit kernel and
ensured there was no degradation in performance.


Khalid Aziz (2):
  Fix CamelCase and extra long lines in the BusLogic driver
  Port buslogic driver to 64-bit.

 drivers/scsi/BusLogic.c   | 4490 -
 drivers/scsi/BusLogic.h   | 1489 ---
 drivers/scsi/FlashPoint.c |  625 +++
 drivers/scsi/Kconfig  |2 +-
 4 files changed, 3395 insertions(+), 3211 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Hard disk S3 resume time optimization

2013-05-16 Thread Brandt, Todd E
Hello, my name's Todd Brandt and I'm the project owner for Intel Open Source 
Technology Center's new project to optimize suspend/resume time.
Our website is: https://01.org/suspendresume

[The Problem]
The vast majority of time spent in S3 resume is consumed by the ATA subsystem 
as it resumes the computer's hard drives. For large hard disks this time can be 
upwards of 10 seconds, which makes S3 suspend/resume too costly to use 
frequently. This time needs to be reduced. I've written a blog entry describing 
the details at this url:
https://01.org/suspendresume/blogs/tebrandt/2013/hard-disk-resume-worst-offender

[Proposed Solution]
I've implemented a potential solution in this patch, which effectively reduces 
total resume time from multiple seconds to less than 700ms. The idea is to 
allow the ATA/SCSI subsystems to resume asynchronously without blocking system 
resume completion. The dpm_resume call currently waits for all asynchronous 
devices to finish resuming before it gives control back to the user. But in the 
case of ATA/SCSI we can give control back immediately, since the hard disks 
won't be needed immediately in lieu of RAM and cache.

Patch1 goes through and sets the power.async_suspend flag for every device in 
the ATA/SCSI resume path. This includes the ata port, link, and dev devices, 
the scsi host and target devices, all their associated transport devices, the 
block devices, and block partitions. This allows the entire ATA resume path to 
be added to the async device queue in drivers/base/power/main.c. Without this, 
the ATA resume path ends up in both queues (sync and async), which causes 
interdependencies and backs up the two queues. It's also needed for the second 
patch to have any effect.

Patch2 updates the drivers/base/power subsystem to allow any devices which have 
registered as asynchronous and who have not registered compete callbacks to 
be non-blocking. As it happens, the ATA subsystem devices don't register 
complete callbacks, so this is a simple way of adding the new functionality and 
getting ATA to conform immediately. 



[PATCH1 - ata_resume_async.patch]

Signed-off-by: Todd Brandt todd.e.bra...@intel.com
---
 block/genhd.c  |2 ++
 block/partition-generic.c  |1 +
 drivers/ata/libata-transport.c |4 +++-
 drivers/base/attribute_container.c |1 +
 drivers/base/core.c|7 ++-
 drivers/scsi/scsi_sysfs.c  |2 +-
 drivers/scsi/sd.c  |3 +++
 7 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 7dcfdd8..8b88c05 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -525,6 +525,8 @@ static void register_disk(struct gendisk *disk)
/* delay uevents, until we scanned partition table */
dev_set_uevent_suppress(ddev, 1);
 
+   device_enable_async_suspend(ddev);
+
if (device_add(ddev))
return;
if (!sysfs_deprecated) {
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 1cb4dec..7f136d1 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -325,6 +325,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int 
partno,
pdev-class = block_class;
pdev-type = part_type;
pdev-parent = ddev;
+   pdev-power.async_suspend = true;
 
err = blk_alloc_devt(p, devt);
if (err)
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index c04d393..493f5ce 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -285,13 +285,13 @@ int ata_tport_add(struct device *parent,
dev-parent = get_device(parent);
dev-release = ata_tport_release;
dev_set_name(dev, ata%d, ap-print_id);
+   device_enable_async_suspend(dev);
transport_setup_device(dev);
error = device_add(dev);
if (error) {
goto tport_err;
}
 
-   device_enable_async_suspend(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
pm_runtime_forbid(dev);
@@ -414,6 +414,7 @@ int ata_tlink_add(struct ata_link *link)
 else
dev_set_name(dev, link%d.%d, ap-print_id, link-pmp);
 
+   device_enable_async_suspend(dev);
transport_setup_device(dev);
 
error = device_add(dev);
@@ -642,6 +643,7 @@ static int ata_tdev_add(struct ata_device *ata_dev)
 else
dev_set_name(dev, dev%d.%d.0, ap-print_id, link-pmp);
 
+   device_enable_async_suspend(dev);
transport_setup_device(dev);
error = device_add(dev);
if (error) {
diff --git a/drivers/base/attribute_container.c 
b/drivers/base/attribute_container.c
index d78b204..49cc02d 100644
--- a/drivers/base/attribute_container.c
+++ b/drivers/base/attribute_container.c
@@ -349,6 +349,7 @@ attribute_container_add_attrs(struct device 

Re: [PATCH] Hard disk S3 resume time optimization

2013-05-16 Thread Tejun Heo
Hello,

First of all, if at all possible, please try to fill the message to 80
columns, preferably a bit narrower than that.  Most email clients can
do it without you knowing.

 [The Problem]
 The vast majority of time spent in S3 resume is consumed by the ATA
 subsystem as it resumes the computer's hard drives. For large hard
 disks this time can be upwards of 10 seconds, which makes S3
 suspend/resume too costly to use frequently. This time needs to be
 reduced. I've written a blog entry describing the details at this
 url:
 https://01.org/suspendresume/blogs/tebrandt/2013/hard-disk-resume-worst-offender

Yeah, this sucks.  Years back, there were several releases in which
libata implemented its own supsend / resume mechanism mostly from the
exception handler, which was fully asynchronous.  It later got
reimplemented in terms of SCSI suspend/resume and lost that, which is
a bummer.

 [Proposed Solution]
 I've implemented a potential solution in this patch, which
 effectively reduces total resume time from multiple seconds to less
 than 700ms. The idea is to allow the ATA/SCSI subsystems to resume
 asynchronously without blocking system resume completion. The
 dpm_resume call currently waits for all asynchronous devices to
 finish resuming before it gives control back to the user. But in the
 case of ATA/SCSI we can give control back immediately, since the
 hard disks won't be needed immediately in lieu of RAM and cache.

Yeap.

So, while I agree about the problem and the solution seems to be
headed the right way of making SCSI suspend/resume asynchronous,
what's going on with patch splitting, submission format and comments?
Please read up on patch submission (there gotta be enough people in
OSTC to point you to the right direction) and retry.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Hard disk S3 resume time optimization

2013-05-16 Thread Tejun Heo
On Thu, May 16, 2013 at 03:44:41PM -0700, Tejun Heo wrote:
 So, while I agree about the problem and the solution seems to be
 headed the right way of making SCSI suspend/resume asynchronous,
 what's going on with patch splitting, submission format and comments?
 Please read up on patch submission (there gotta be enough people in
 OSTC to point you to the right direction) and retry.

Please also cc Rafael J. Wysocki r...@sisk.pl and
linux...@vger.kernel.org next time.

Thanks!

-- 
tejun
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] scsi: Port buslogic driver to 64 bits

2013-05-16 Thread Khalid Aziz
This patchset ports buslogic driver to 64-bit.

Current buslogic driver is composed of two components - SCCB manager
which communicates with adapter to execute SCSI commands (contained in
FlashPoint.c), and Linux driver part that interfaces with rest of the
kernel (contained in BusLogic.c). SCCB manager code came from the
Mylex SDK. SCCB manager code is used by flashpoint adapters only.
Multimaster adapters do not need SCCB manager since the SCSI state
machine is implemented in firmware on these adapters. If the filenames
alone did not give it away already, buslogic driver code is full of
CamelCase, besides being full of very ling lines, and is just very
difficult to read and understand. So the first step was to clean up
the existing code. First patch in the set does just that. Second patch
includes necessary code modifications to allow the driver to build and
run on 64-bit kernel. Since SCCB manager code came from Mylex SDK, I
have tried to touch it only when necessary which includes not fixing
all CamelCase issues in FlashPoint.c. Many lines over 80 characters
remain in BusLogic.c. These fall into two categories generally -
(1) it prints a message and I didn't want to touch driver messages
in case there are scripts out there that parse driver messages,
(2) code is indented deeply and is hard to keep it under 80 characters.
Such code could use refactoring at some point.

I have tested this patch with a flashpoint adapter on 64-bit and 
32-bit kernels with fio running random read/write test while verifying
data. I also measured performance for current buslogic driver and 
buslogic driver with these patches with 32-bit and 64-bit kernel and
ensured there was no degradation in performance.

Changelog:
v2:
- Updated to apply on top of current Linus' tree as of May 16, 2013. 
  No functional changes.

Khalid Aziz (2):
  Fix CamelCase and extra long lines in the buslogic driver.
  Port buslogic driver to 64-bit.

 drivers/scsi/BusLogic.c   | 4452 -
 drivers/scsi/BusLogic.h   | 1487 ---
 drivers/scsi/FlashPoint.c |  626 +++
 drivers/scsi/Kconfig  |2 +-
 4 files changed, 3377 insertions(+), 3190 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] csiostor: Retain default adapter configuration in absence of config file.

2013-05-16 Thread Naresh Kumar Inna
- Retain firmware defined configuration settings in the absence of
user-provided configuration by eliminating the global RSS and the
PF/VF capabilities mailbox commands.
- Remove S_IRUGO from sysfs parameters that don't have 'show'
functionality.

Signed-off-by: Naresh Kumar Inna nar...@chelsio.com
---
 drivers/scsi/csiostor/csio_hw.c   |   91 -
 drivers/scsi/csiostor/csio_hw.h   |   11 -
 drivers/scsi/csiostor/csio_mb.c   |   77 ---
 drivers/scsi/csiostor/csio_mb.h   |   11 -
 drivers/scsi/csiostor/csio_scsi.c |4 +-
 5 files changed, 2 insertions(+), 192 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 1936055..0eb35b9 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -1597,87 +1597,6 @@ out:
return rv;
 }
 
-static int
-csio_config_global_rss(struct csio_hw *hw)
-{
-   struct csio_mb  *mbp;
-   enum fw_retval retval;
-
-   mbp = mempool_alloc(hw-mb_mempool, GFP_ATOMIC);
-   if (!mbp) {
-   CSIO_INC_STATS(hw, n_err_nomem);
-   return -ENOMEM;
-   }
-
-   csio_rss_glb_config(hw, mbp, CSIO_MB_DEFAULT_TMO,
-   FW_RSS_GLB_CONFIG_CMD_MODE_BASICVIRTUAL,
-   FW_RSS_GLB_CONFIG_CMD_TNLMAPEN |
-   FW_RSS_GLB_CONFIG_CMD_HASHTOEPLITZ |
-   FW_RSS_GLB_CONFIG_CMD_TNLALLLKP,
-   NULL);
-
-   if (csio_mb_issue(hw, mbp)) {
-   csio_err(hw, Issue of FW_RSS_GLB_CONFIG_CMD failed!\n);
-   mempool_free(mbp, hw-mb_mempool);
-   return -EINVAL;
-   }
-
-   retval = csio_mb_fw_retval(mbp);
-   if (retval != FW_SUCCESS) {
-   csio_err(hw, FW_RSS_GLB_CONFIG_CMD returned 0x%x!\n, retval);
-   mempool_free(mbp, hw-mb_mempool);
-   return -EINVAL;
-   }
-
-   mempool_free(mbp, hw-mb_mempool);
-
-   return 0;
-}
-
-/*
- * csio_config_pfvf - Configure Physical/Virtual functions settings.
- * @hw: HW module
- *
- */
-static int
-csio_config_pfvf(struct csio_hw *hw)
-{
-   struct csio_mb  *mbp;
-   enum fw_retval retval;
-
-   mbp = mempool_alloc(hw-mb_mempool, GFP_ATOMIC);
-   if (!mbp) {
-   CSIO_INC_STATS(hw, n_err_nomem);
-   return -ENOMEM;
-   }
-
-   /*
-* For now, allow all PFs to access to all ports using a pmask
-* value of 0xF (M_FW_PFVF_CMD_PMASK). Once we have VFs, we will
-* need to provide access based on some rule.
-*/
-   csio_mb_pfvf(hw, mbp, CSIO_MB_DEFAULT_TMO, hw-pfn, 0, CSIO_NEQ,
-CSIO_NETH_CTRL, CSIO_NIQ_FLINT, 0, 0, CSIO_NVI, CSIO_CMASK,
-CSIO_PMASK, CSIO_NEXACTF, CSIO_R_CAPS, CSIO_WX_CAPS, NULL);
-
-   if (csio_mb_issue(hw, mbp)) {
-   csio_err(hw, Issue of FW_PFVF_CMD failed!\n);
-   mempool_free(mbp, hw-mb_mempool);
-   return -EINVAL;
-   }
-
-   retval = csio_mb_fw_retval(mbp);
-   if (retval != FW_SUCCESS) {
-   csio_err(hw, FW_PFVF_CMD returned 0x%x!\n, retval);
-   mempool_free(mbp, hw-mb_mempool);
-   return -EINVAL;
-   }
-
-   mempool_free(mbp, hw-mb_mempool);
-
-   return 0;
-}
-
 /*
  * csio_enable_ports - Bring up all available ports.
  * @hw: HW module.
@@ -2056,16 +1975,6 @@ csio_hw_no_fwconfig(struct csio_hw *hw, int reset)
if (rv != 0)
goto out;
 
-   /* Config Global RSS command */
-   rv = csio_config_global_rss(hw);
-   if (rv != 0)
-   goto out;
-
-   /* Configure PF/VF capabilities of device */
-   rv = csio_config_pfvf(hw);
-   if (rv != 0)
-   goto out;
-
/* device parameters */
rv = csio_get_device_params(hw);
if (rv != 0)
diff --git a/drivers/scsi/csiostor/csio_hw.h b/drivers/scsi/csiostor/csio_hw.h
index 489fc09..49b1daa 100644
--- a/drivers/scsi/csiostor/csio_hw.h
+++ b/drivers/scsi/csiostor/csio_hw.h
@@ -153,17 +153,6 @@ enum {
CSIO_SGE_INT_CNT_VAL_1  = 4,
CSIO_SGE_INT_CNT_VAL_2  = 8,
CSIO_SGE_INT_CNT_VAL_3  = 16,
-
-   /* Storage specific - used by FW_PFVF_CMD */
-   CSIO_WX_CAPS= FW_CMD_CAP_PF, /* w/x all */
-   CSIO_R_CAPS = FW_CMD_CAP_PF, /* r all */
-   CSIO_NVI= 4,
-   CSIO_NIQ_FLINT  = 34,
-   CSIO_NETH_CTRL  = 32,
-   CSIO_NEQ= 66,
-   CSIO_NEXACTF= 32,
-   CSIO_CMASK  = FW_PFVF_CMD_CMASK_MASK,
-   CSIO_PMASK  = FW_PFVF_CMD_PMASK_MASK,
 };
 
 /* Slowpath events */
diff --git a/drivers/scsi/csiostor/csio_mb.c b/drivers/scsi/csiostor/csio_mb.c
index f5d9ee1..15b6351 

[PATCH 0/3] BusLogic: Message logging neatening

2013-05-16 Thread Joe Perches
If you're going through the trouble to fix this CamelCase stuff
and make it work on 64 bit, how about a little more cleanup?

Joe Perches (3):
  BusLogic: Add __printf verification, fix fallout
  BusLogic: Coalesce formats with multiple string fragments
  BusLogic: Use more conventional argument order for logging

 drivers/scsi/BusLogic.c | 304 
 drivers/scsi/BusLogic.h |  23 ++--
 2 files changed, 161 insertions(+), 166 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] BusLogic: Add __printf verification, fix fallout

2013-05-16 Thread Joe Perches
Make format and arguments match.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/scsi/BusLogic.c | 50 -
 drivers/scsi/BusLogic.h |  1 +
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index feab3a5..28c2051 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -720,23 +720,23 @@ static int __init blogic_init_mm_probeinfo(struct 
blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 
if (pci_resource_flags(pci_device, 0)  IORESOURCE_MEM) {
-   blogic_err(BusLogic: Base Address0 0x%X not I/O for  
MultiMaster Host Adapter\n, NULL, base_addr0);
-   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%X\n, NULL, bus, device, io_addr);
+   blogic_err(BusLogic: Base Address0 0x%lX not I/O for  
MultiMaster Host Adapter\n, NULL, base_addr0);
+   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1)  IORESOURCE_IO) {
-   blogic_err(BusLogic: Base Address1 0x%X not Memory for 
 MultiMaster Host Adapter\n, NULL, base_addr1);
-   blogic_err(at PCI Bus %d Device %d PCI Address 
0x%X\n, NULL, bus, device, pci_addr);
+   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for  MultiMaster Host Adapter\n, NULL, base_addr1);
+   blogic_err(at PCI Bus %d Device %d PCI Address 
0x%lX\n, NULL, bus, device, pci_addr);
continue;
}
if (irq_ch == 0) {
blogic_err(BusLogic: IRQ Channel %d invalid for  
MultiMaster Host Adapter\n, NULL, irq_ch);
-   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%X\n, NULL, bus, device, io_addr);
+   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (blogic_global_options.trace_probe) {
blogic_notice(BusLogic: PCI MultiMaster Host Adapter  
detected at\n, NULL);
-   blogic_notice(BusLogic: PCI Bus %d Device %d I/O 
Address  0x%X PCI Address 0x%X\n, NULL, bus, device, io_addr, pci_addr);
+   blogic_notice(BusLogic: PCI Bus %d Device %d I/O 
Address 0x%lX PCI Address 0x%lX\n, NULL, bus, device, io_addr, pci_addr);
}
/*
   Issue the Inquire PCI Host Adapter Information command to 
determine
@@ -960,23 +960,23 @@ static int __init blogic_init_fp_probeinfo(struct 
blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 #ifdef CONFIG_SCSI_FLASHPOINT
if (pci_resource_flags(pci_device, 0)  IORESOURCE_MEM) {
-   blogic_err(BusLogic: Base Address0 0x%X not I/O for  
FlashPoint Host Adapter\n, NULL, base_addr0);
-   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%X\n, NULL, bus, device, io_addr);
+   blogic_err(BusLogic: Base Address0 0x%lX not I/O for  
FlashPoint Host Adapter\n, NULL, base_addr0);
+   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1)  IORESOURCE_IO) {
-   blogic_err(BusLogic: Base Address1 0x%X not Memory for 
 FlashPoint Host Adapter\n, NULL, base_addr1);
-   blogic_err(at PCI Bus %d Device %d PCI Address 
0x%X\n, NULL, bus, device, pci_addr);
+   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for  FlashPoint Host Adapter\n, NULL, base_addr1);
+   blogic_err(at PCI Bus %d Device %d PCI Address 
0x%lX\n, NULL, bus, device, pci_addr);
continue;
}
if (irq_ch == 0) {
blogic_err(BusLogic: IRQ Channel %d invalid for  
FlashPoint Host Adapter\n, NULL, irq_ch);
-   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%X\n, NULL, bus, device, io_addr);
+   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (blogic_global_options.trace_probe) {
blogic_notice(BusLogic: FlashPoint Host Adapter  
detected at\n, NULL);
-   blogic_notice(BusLogic: PCI Bus %d Device %d I/O 
Address  0x%X PCI Address 0x%X\n, NULL, bus, device, io_addr, pci_addr);
+   blogic_notice(BusLogic: PCI Bus %d Device %d I/O 
Address 0x%lX PCI Address 

[PATCH 2/3] BusLogic: Coalesce formats with multiple string fragments

2013-05-16 Thread Joe Perches
Strings fragments coalesced by the compiler are
difficult to grep.  Coalesce them instead.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/scsi/BusLogic.c | 66 -
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 28c2051..bd588cf 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -141,7 +141,7 @@ static char *blogic_cmd_failure_reason;
 static void blogic_announce_drvr(struct blogic_adapter *adapter)
 {
blogic_announce(* BusLogic SCSI Driver Version  
blogic_drvr_version  of  blogic_drvr_date  *\n, adapter);
-   blogic_announce(Copyright 1995-1998 by Leonard N. Zubkoff  
l...@dandelion.com\n, adapter);
+   blogic_announce(Copyright 1995-1998 by Leonard N. Zubkoff 
l...@dandelion.com\n, adapter);
 }
 
 
@@ -444,7 +444,7 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum 
blogic_opcode opcode,
goto done;
}
if (blogic_global_options.trace_config)
-   blogic_notice(blogic_cmd(%02X) Status = %02X:  
(Modify I/O Address)\n, adapter, opcode, statusreg.all);
+   blogic_notice(blogic_cmd(%02X) Status = %02X: (Modify 
I/O Address)\n, adapter, opcode, statusreg.all);
result = 0;
goto done;
}
@@ -720,22 +720,22 @@ static int __init blogic_init_mm_probeinfo(struct 
blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 
if (pci_resource_flags(pci_device, 0)  IORESOURCE_MEM) {
-   blogic_err(BusLogic: Base Address0 0x%lX not I/O for  
MultiMaster Host Adapter\n, NULL, base_addr0);
+   blogic_err(BusLogic: Base Address0 0x%lX not I/O for 
MultiMaster Host Adapter\n, NULL, base_addr0);
blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1)  IORESOURCE_IO) {
-   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for  MultiMaster Host Adapter\n, NULL, base_addr1);
+   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for MultiMaster Host Adapter\n, NULL, base_addr1);
blogic_err(at PCI Bus %d Device %d PCI Address 
0x%lX\n, NULL, bus, device, pci_addr);
continue;
}
if (irq_ch == 0) {
-   blogic_err(BusLogic: IRQ Channel %d invalid for  
MultiMaster Host Adapter\n, NULL, irq_ch);
+   blogic_err(BusLogic: IRQ Channel %d invalid for 
MultiMaster Host Adapter\n, NULL, irq_ch);
blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (blogic_global_options.trace_probe) {
-   blogic_notice(BusLogic: PCI MultiMaster Host Adapter  
detected at\n, NULL);
+   blogic_notice(BusLogic: PCI MultiMaster Host Adapter 
detected at\n, NULL);
blogic_notice(BusLogic: PCI Bus %d Device %d I/O 
Address 0x%lX PCI Address 0x%lX\n, NULL, bus, device, io_addr, pci_addr);
}
/*
@@ -822,7 +822,7 @@ static int __init blogic_init_mm_probeinfo(struct 
blogic_adapter *adapter)
nonpr_mmcount++;
mmcount++;
} else
-   blogic_warn(BusLogic: Too many Host Adapters  
detected\n, NULL);
+   blogic_warn(BusLogic: Too many Host Adapters 
detected\n, NULL);
}
/*
   If the AutoSCSI Use Bus And Device # For PCI Scanning Seq.
@@ -960,22 +960,22 @@ static int __init blogic_init_fp_probeinfo(struct 
blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 #ifdef CONFIG_SCSI_FLASHPOINT
if (pci_resource_flags(pci_device, 0)  IORESOURCE_MEM) {
-   blogic_err(BusLogic: Base Address0 0x%lX not I/O for  
FlashPoint Host Adapter\n, NULL, base_addr0);
+   blogic_err(BusLogic: Base Address0 0x%lX not I/O for 
FlashPoint Host Adapter\n, NULL, base_addr0);
blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
continue;
}
if (pci_resource_flags(pci_device, 1)  IORESOURCE_IO) {
-   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for  FlashPoint Host Adapter\n, NULL, base_addr1);
+   blogic_err(BusLogic: Base Address1 0x%lX not Memory 
for FlashPoint Host Adapter\n, NULL, base_addr1);
blogic_err(at PCI Bus %d Device %d PCI Address 

[PATCH 3/3] BusLogic: Use more conventional argument order for logging

2013-05-16 Thread Joe Perches
Subsystem specific logging messages generally use
subsystem_level(struct subsystem *, fmt, ...)
not
subsystem_level(fmt, struct subsystem *, ...)

Convert to use the more generally used kernel style.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/scsi/BusLogic.c | 304 
 drivers/scsi/BusLogic.h |  24 ++--
 2 files changed, 161 insertions(+), 167 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index bd588cf..d5a5d97 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -140,8 +140,8 @@ static char *blogic_cmd_failure_reason;
 
 static void blogic_announce_drvr(struct blogic_adapter *adapter)
 {
-   blogic_announce(* BusLogic SCSI Driver Version  
blogic_drvr_version  of  blogic_drvr_date  *\n, adapter);
-   blogic_announce(Copyright 1995-1998 by Leonard N. Zubkoff 
l...@dandelion.com\n, adapter);
+   blogic_announce(adapter, * BusLogic SCSI Driver Version  
blogic_drvr_version  of  blogic_drvr_date  *\n);
+   blogic_announce(adapter, Copyright 1995-1998 by Leonard N. Zubkoff 
l...@dandelion.com\n);
 }
 
 
@@ -204,8 +204,7 @@ static bool __init blogic_create_initccbs(struct 
blogic_adapter *adapter)
blk_pointer = pci_alloc_consistent(adapter-pci_device,
blk_size, blkp);
if (blk_pointer == NULL) {
-   blogic_err(UNABLE TO ALLOCATE CCB GROUP - DETACHING\n,
-   adapter);
+   blogic_err(adapter, UNABLE TO ALLOCATE CCB GROUP - 
DETACHING\n);
return false;
}
blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
@@ -264,10 +263,10 @@ static void blogic_create_addlccbs(struct blogic_adapter 
*adapter,
}
if (adapter-alloc_ccbs  prev_alloc) {
if (print_success)
-   blogic_notice(Allocated %d additional CCBs (total now 
%d)\n, adapter, adapter-alloc_ccbs - prev_alloc, adapter-alloc_ccbs);
+   blogic_notice(adapter, Allocated %d additional CCBs 
(total now %d)\n, adapter-alloc_ccbs - prev_alloc, adapter-alloc_ccbs);
return;
}
-   blogic_notice(Failed to allocate additional CCBs\n, adapter);
+   blogic_notice(adapter, Failed to allocate additional CCBs\n);
if (adapter-drvr_qdepth  adapter-alloc_ccbs - adapter-tgt_count) {
adapter-drvr_qdepth = adapter-alloc_ccbs - adapter-tgt_count;
adapter-scsi_host-can_queue = adapter-drvr_qdepth;
@@ -444,7 +443,7 @@ static int blogic_cmd(struct blogic_adapter *adapter, enum 
blogic_opcode opcode,
goto done;
}
if (blogic_global_options.trace_config)
-   blogic_notice(blogic_cmd(%02X) Status = %02X: (Modify 
I/O Address)\n, adapter, opcode, statusreg.all);
+   blogic_notice(adapter, blogic_cmd(%02X) Status = %02X: 
(Modify I/O Address)\n, opcode, statusreg.all);
result = 0;
goto done;
}
@@ -502,15 +501,15 @@ static int blogic_cmd(struct blogic_adapter *adapter, 
enum blogic_opcode opcode,
 */
if (blogic_global_options.trace_config) {
int i;
-   blogic_notice(blogic_cmd(%02X) Status = %02X: %2d == %2d:,
-   adapter, opcode, statusreg.all, replylen,
-   reply_b);
+   blogic_notice(adapter, blogic_cmd(%02X) Status = %02X: %2d == 
%2d:,
+ opcode, statusreg.all, replylen,
+ reply_b);
if (replylen  reply_b)
replylen = reply_b;
for (i = 0; i  replylen; i++)
-   blogic_notice( %02X, adapter,
-   ((unsigned char *) reply)[i]);
-   blogic_notice(\n, adapter);
+   blogic_notice(adapter,  %02X,
+ ((unsigned char *) reply)[i]);
+   blogic_notice(adapter, \n);
}
/*
   Process Command Invalid conditions.
@@ -720,23 +719,23 @@ static int __init blogic_init_mm_probeinfo(struct 
blogic_adapter *adapter)
pci_addr = base_addr1 = pci_resource_start(pci_device, 1);
 
if (pci_resource_flags(pci_device, 0)  IORESOURCE_MEM) {
-   blogic_err(BusLogic: Base Address0 0x%lX not I/O for 
MultiMaster Host Adapter\n, NULL, base_addr0);
-   blogic_err(at PCI Bus %d Device %d I/O Address 
0x%lX\n, NULL, bus, device, io_addr);
+   blogic_err(NULL, BusLogic: Base Address0 0x%lX not I/O 
for MultiMaster Host Adapter\n, base_addr0);
+   blogic_err(NULL, at PCI Bus %d Device %d I/O Address 

Re: [PATCH] qla2x00t: Fix a memory leak in an error path

2013-05-16 Thread Saurav Kashyap
Hi Bart,

Instead of doing this I would move this check to the top of the function,
something like this
-8--
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 371bb86..b5f84ae 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -255,6 +255,12 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
int rval =  (DRIVER_ERROR  16);
uint16_t nextlid = 0;

 
+   if (!vha-flags.online) {
+   ql_log(ql_log_warn, vha, 0x7005, Host not online.\n);
+   rval = -EIO;
+   goto done;
+   }
+
if (bsg_job-request-msgcode == FC_BSG_RPT_ELS) {
rport = bsg_job-rport;
fcport = *(fc_port_t **) rport-dd_data;
@@ -326,12 +332,6 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
NPH_FABRIC_CONTROLLER : NPH_F_PORT;
}

 
-   if (!vha-flags.online) {
-   ql_log(ql_log_warn, vha, 0x7005, Host not online.\n);
-   rval = -EIO;
-   goto done;
-   }
-
req_sg_cnt =
dma_map_sg(ha-pdev-dev,
bsg_job-request_payload.sg_list,
bsg_job-request_payload.sg_cnt, DMA_TO_DEVICE);

8-

Thanks,
~Saurav




Avoid that the fcport structure gets leaked if
bsg_job-request-msgcode == FC_BSG_HST_ELS_NOLOGIN, the fcport
allocation succeeds and the !vha-flags.online branch is taken.
Detected by Coverity.

Signed-off-by: Bart Van Assche bvanass...@acm.org
Cc: Chad Dupuis chad.dup...@qlogic.com
Cc: Saurav Kashyap saurav.kash...@qlogic.com
---
 drivers/scsi/qla2xxx/qla_bsg.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c
b/drivers/scsi/qla2xxx/qla_bsg.c
index 39719f8..af35707 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -329,7 +329,7 @@ qla2x00_process_els(struct fc_bsg_job *bsg_job)
   if (!vha-flags.online) {
   ql_log(ql_log_warn, vha, 0x7005, Host not online.\n);
   rval = -EIO;
-  goto done;
+  goto done_free_fcport;
   }
 
   req_sg_cnt =
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

attachment: winmail.dat