ibmvscsi: dynamic request_limit and device restart
Subsystems (3)
cover ibmvscsi: dynamic request_limit and device restart
James,
Resending these patches for inclusion in your tree.
There are two fixes for the ibmvscsi client driver in this set.
- Dynamic request_limit
The request_limit for the driver was not properly reflecting the value on the server side and could cause can_queue to be set to improper values (-1). The patch corrects this so that request_limit mirrors the value on the server and sets can_queue appropriately.
- Device restart
When a drive was removed from the server and then re-added the client would not be able to use that device. The device would return a unit attention and then not ready. By adding a slave_configure function we can set the "allow_restart" flag for all disk devices. Now devices will resume functioning when they are re-added to the server.
---
PATCH 1/2 ibmvscsi: allow for dynamic adjustment of server request_limit
The request limit calculations used previously on the client failed to mirror the state of the server. Additionally, when a value < 3 was provided there could be problems setting can_queue and handling abort and reset commands.
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 58 +++++++++++++++++++++++++-------------- drivers/scsi/ibmvscsi/ibmvscsi.h | 2 + 2 files changed, 40 insertions(+), 20 deletions(-)
drivers/scsi/ibmvscsi/ibmvscsi.c +38 -20
@@ -85,7 +85,7 @@static int max_id =64 ;static int max_channel =3 ;static int init_timeout =5 ; -static int max_requests =50 ; +static int max_requests =IBMVSCSI_MAX_REQUESTS_DEFAULT ;#define IBMVSCSI_VERSION "1.5.8"
@@ -538,7 +538,8 @@int request_status;int rc; -/* If we have exhausted our request limit, just fail this request. +/* If we have exhausted our request limit, just fail this request, +* unless it is for a reset or abort. * Note that there are rare cases involving driver generated requests * (such as task management requests) that the mid layer may think we * can handle more requests (can_queue) when we actually can't
@@ -551,9 +552,30 @@*/ if (request_status <-1 ) goto send_error; -/* Otherwise, if we have run out of requests */ -else if (request_status <0 ) - goto send_busy; +/* Otherwise, we may have run out of requests. */ +/* Abort and reset calls should make it through. +* Nothing except abort and reset should use the last two +* slots unless we had two or less to begin with. +*/ +else if (request_status <2 && + evt_struct->iu.srp.cmd.opcode !=SRP_TSK_MGMT ) { +/* In the case that we have less than two requests +* available, check the server limit as a combination +* of the request limit and the number of requests +* in-flight (the size of the send list). If the +* server limit is greater than 2, return busy so +* that the last two are reserved for reset and abort. +*/ +int server_limit = request_status; +struct srp_event_struct *tmp_evt; + +list_for_each_entry (tmp_evt, &hostdata->sent, list) { + server_limit++; + } + +if (server_limit >2 ) + goto send_busy; + } }/* Copy the IU into the transfer area */
@@ -572,6 +594,7 @@printk (KERN_ERR "ibmvscsi: send error %d\n" , rc); +atomic_inc (&hostdata->request_limit); goto send_error; }
@@ -581,7 +604,8 @@unmap_cmd_data (&evt_struct->iu.srp.cmd, evt_struct, hostdata->dev);free_event_struct (&hostdata->pool, evt_struct); -return SCSI_MLQUEUE_HOST_BUSY ; +atomic_inc (&hostdata->request_limit); +return SCSI_MLQUEUE_HOST_BUSY ; send_error:unmap_cmd_data (&evt_struct->iu.srp.cmd, evt_struct, hostdata->dev);
@@ -831,23 +855,16 @@printk (KERN_INFO "ibmvscsi: SRP_LOGIN succeeded\n" ); -if (evt_struct->xfer_iu->srp.login_rsp.req_lim_delta > - (max_requests -2 )) - evt_struct->xfer_iu->srp.login_rsp.req_lim_delta = - max_requests -2 ; +if (evt_struct->xfer_iu->srp.login_rsp.req_lim_delta <0 ) +printk (KERN_ERR "ibmvscsi: Invalid request_limit.\n" ); -/* Now we know what the real request-limit is */ +/* Now we know what the real request-limit is. +* This value is set rather than added to request_limit because +* request_limit could have been set to -1 by this client. +*/ atomic_set (&hostdata->request_limit, evt_struct->xfer_iu->srp.login_rsp.req_lim_delta); - hostdata->host->can_queue = - evt_struct->xfer_iu->srp.login_rsp.req_lim_delta -2 ; - -if (hostdata->host->can_queue <1 ) { -printk (KERN_ERR "ibmvscsi: Invalid request_limit_delta\n" ); -return ; - } -/* If we had any pending I/Os, kick them */ scsi_unblock_requests (hostdata->host);
@@ -1483,7 +1500,7 @@ .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, .cmd_per_lun = 16, - .can_queue = 1, /* Updated after SRP_LOGIN */ + .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1, .sg_tablesize = SG_ALL, .use_clustering = ENABLE_CLUSTERING,
@@ -1503,6 +1520,7 @@ vdev->dev.driver_data =NULL ; + driver_template.can_queue = max_requests; host =scsi_host_alloc (&driver_template,sizeof (*hostdata));if (!host) {printk (KERN_ERR "ibmvscsi: couldn't allocate host data\n" );
drivers/scsi/ibmvscsi/ibmvscsi.h +2 -0
@@ -44,6 +44,8 @@*/ #define MAX_INDIRECT_BUFS 10 +#define IBMVSCSI_MAX_REQUESTS_DEFAULT 100 +/* ------------------------------------------------------------ * Data Structures */
PATCH 2/2 ibmvscsi: add slave_configure to allow device restart
In reply to Robert JenningsAdding a slave_configure function for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/scsi/ibmvscsi/ibmvscsi.c +18 -0
@@ -1354,6 +1354,23 @@return rc; } +/** +* ibmvscsi_slave_configure: For each slave device that is a disk, +* ensure that the "allow_restart" flag is enabled. +*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags); +if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
@@ -1499,6 +1516,7 @@ .queuecommand = ibmvscsi_queuecommand, .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, + .slave_configure = ibmvscsi_slave_configure, .cmd_per_lun = 16, .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1,
Re: [PATCH 2/2] ibmvscsi: add slave_configure to allow device restart
Quoted 15 lines · On Wed, 28 Mar 2007 12:47:04 -0500 Robert Jennings wrote:
Adding a
slave_configurefunction for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.Signed-off-by "Robert Jennings" <[email protected]>Signed-off-by "Santiago Leon" <[email protected]>--- drivers/scsi/ibmvscsi/ibmvscsi.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)Index:
b/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================
drivers/scsi/ibmvscsi/ibmvscsi.c line 1359
Quoted 6 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +3
-0@@ -1354,6 +1354,23 @@return rc; } +/** +* ibmvscsi_slave_configure: For each slave device that is a disk, +* ensure that the "allow_restart" flag is enabled.
Hi,
Please don't use kernel-doc notation (/**) unless the following documentation block is in kernel-doc format. See Documentation/kernel-doc-nano-HOWTO.txt for more info, or just ask me questions if you have any.
Quoted 18 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +14
-0+*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags); +if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
drivers/scsi/ibmvscsi/ibmvscsi.c line 1522
Quoted 7 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +1
-0@@ -1499,6 +1516,7 @@ .queuecommand = ibmvscsi_queuecommand, .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, + .slave_configure = ibmvscsi_slave_configure, .cmd_per_lun = 16, .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Re: [PATCH 2/2] ibmvscsi: add slave_configure to allow device restart
Fixed the kernel-doc comment for ibmvscsi_slave_configure. Thanks to Randy Dunlap for catching that.
Adding a slave_configure function for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
drivers/scsi/ibmvscsi/ibmvscsi.c +23 -0
@@ -1354,6 +1354,28 @@return rc; } +/** +* ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk. +* +* @sdev: struct scsi_device device to configure +* +* Enable allow_restart for a device if it is a disk. Adjust the +* queue_depth here also as is required by the documentation for +* struct scsi_host_template. +*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags); +if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
@@ -1499,6 +1521,7 @@ .queuecommand = ibmvscsi_queuecommand, .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, + .slave_configure = ibmvscsi_slave_configure, .cmd_per_lun = 16, .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1,
Re: [PATCH 2/2] ibmvscsi: add slave_configure to allow device restart
Quoted 2 lines · On Thu, 29 Mar 2007 11:28:36 -0500 Robert Jennings wrote:
Fixed the kernel-doc comment for
ibmvscsi_slave_configure. Thanks to Randy Dunlap for catching that.
Sorry, there's still a minor nit below.
Quoted 15 lines
Adding a
slave_configurefunction for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.Signed-off-by "Robert Jennings" <[email protected]>Signed-off-by "Santiago Leon" <[email protected]>--- drivers/scsi/ibmvscsi/ibmvscsi.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)Index:
b/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================
drivers/scsi/ibmvscsi/ibmvscsi.c line 1359
Quoted 6 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +3
-0@@ -1354,6 +1354,28 @@return rc; } +/** +* ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk. +*
No "blank" ("*") line between the function name and its parameters... it mucks up the generated output.
Quoted 23 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +19
-0+* @sdev: struct scsi_device device to configure +* +* Enable allow_restart for a device if it is a disk. Adjust the +* queue_depth here also as is required by the documentation for +* struct scsi_host_template. +*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags); +if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
drivers/scsi/ibmvscsi/ibmvscsi.c line 1527
Quoted 7 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +1
-0@@ -1499,6 +1521,7 @@ .queuecommand = ibmvscsi_queuecommand, .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, + .slave_configure = ibmvscsi_slave_configure, .cmd_per_lun = 16, .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
PATCH v3 2/2 ibmvscsi: add slave_configure to allow device restart
Fixed the kernel-doc comment for ibmvscsi_slave_configure. Thanks to Randy Dunlap for pointing this out.
Adding a slave_configure function for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
drivers/scsi/ibmvscsi/ibmvscsi.c +22 -0
@@ -1354,6 +1354,27 @@return rc; } +/** +* ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk. +* @sdev: struct scsi_device device to configure +* +* Enable allow_restart for a device if it is a disk. Adjust the +* queue_depth here also as is required by the documentation for +* struct scsi_host_template. +*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags); +if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
@@ -1499,6 +1520,7 @@ .queuecommand = ibmvscsi_queuecommand, .eh_abort_handler = ibmvscsi_eh_abort_handler, .eh_device_reset_handler = ibmvscsi_eh_device_reset_handler, + .slave_configure = ibmvscsi_slave_configure, .cmd_per_lun = 16, .can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT, .this_id = -1,
Re: [PATCH 2/2][v3] ibmvscsi: add slave_configure to allow device restart
Quoted 8 lines · On Thu, 29 Mar 2007 12:30:40 -0500 Robert Jennings wrote:
Fixed the kernel-doc comment for
ibmvscsi_slave_configure. Thanks to Randy Dunlap for pointing this out.Adding a
slave_configurefunction for the driver. Now the disks can be restarted by the scsi mid-layer when the are disconnected and reconnected.Signed-off-by "Robert Jennings" <[email protected]>Signed-off-by "Santiago Leon" <[email protected]>
Looks good, thanks.
Quoted 26 lines
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)Index:
b/drivers/scsi/ibmvscsi/ibmvscsi.c
===================================================================drivers/scsi/ibmvscsi/ibmvscsi.c +14
-0@@ -1354,6 +1354,27 @@return rc; } +/** +* ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk. +* @sdev: struct scsi_device device to configure +* +* Enable allow_restart for a device if it is a disk. Adjust the +* queue_depth here also as is required by the documentation for +* struct scsi_host_template. +*/ +static int ibmvscsi_slave_configure (struct scsi_device *sdev) +{ +struct Scsi_Host *shost = sdev->host; +unsigned long lock_flags =0 ; + +spin_lock_irqsave (shost->host_lock, lock_flags);
drivers/scsi/ibmvscsi/ibmvscsi.c line 1380
Quoted 10 lines
drivers/scsi/ibmvscsi/ibmvscsi.c +7
-0+if (sdev->type ==TYPE_DISK ) + sdev->allow_restart =1 ; +scsi_adjust_queue_depth (sdev,0 , shost->cmd_per_lun); +spin_unlock_irqrestore (shost->host_lock, lock_flags); +return 0 ; +} +/* ------------------------------------------------------------ * sysfs attributes */
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***