[PATCH scsi-misc-2.6 00/03] scsi: misc timer fixes (again)

[PATCH scsi-misc-2.6 00/03] scsi: misc timer fixes (again)

[Tejun Heo]

 Hello, James.

 It's been a while, but I'm finally settled with git. :-)

 This is repost of the previous scsi timer patchset.  After thinking
about it a while, the first patch seemed unnecessary as you told, so
it's dropped and the others are regenerated.

 aic79xx_osm.c DV still uses eh_timeout.  Is it gonna be updated like
aic7xxx_osm.c is updated?  If not, I have a patch to fix the eh timer
part.  I have a patcheset waiting for it to be clared - unexporting
SCSI timer as the semantics is very specific and nothing good can come
from tempering with it.  Once aic79xx_osm.c is cleared, I'll post the
patches.

[ Start of patch descriptions ]

01_scsi_timer_dispatch_race_fix.patch
	: remove a timer race in scsi_queue_insert()

	scsi_queue_insert() has four callers.  Three callers call with
	timer disabled and one (the second invocation in
	scsi_dispatch_cmd()) calls with timer activated.
	scsi_queue_insert() used to always call scsi_delete_timer()
	and ignore the return value.  This results in race with timer
	expiration.  Remove scsi_delete_timer() call from
	scsi_queue_insert() and make the caller delete timer and check
	the return value.

02_scsi_timer_remove_delete_timer_from_reset_provider.patch
	: remove unnecessary scsi_delete_timer() call in scsi_reset_provider()

	scsi_reset_provider() calls scsi_delete_timer() on exit which
	isn't necessary.  Remove it.

03_scsi_timer_eh_timer_remove_spurious_if.patch
	: remove spurious if tests from scsi_eh_{times_out|done}

	'if' tests which check if eh_action isn't NULL in both
	functions are always true.  Remove the redundant if's as it
	can give wrong impressions.

[ End of patch descriptions ]

 Thanks.

Re: [PATCH scsi-misc-2.6 01/03] scsi: remove a timer race in scsi_queue_insert()

[Tejun Heo]

01_scsi_timer_dispatch_race_fix.patch

	scsi_queue_insert() has four callers.  Three callers call with
	timer disabled and one (the second invocation in
	scsi_dispatch_cmd()) calls with timer activated.
	scsi_queue_insert() used to always call scsi_delete_timer()
	and ignore the return value.  This results in race with timer
	expiration.  Remove scsi_delete_timer() call from
	scsi_queue_insert() and make the caller delete timer and check
	the return value.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 scsi.c     |   10 ++++++----
 scsi_lib.c |    8 +-------
 2 files changed, 7 insertions(+), 11 deletions(-)

Index: scsi-reqfn-export/drivers/scsi/scsi.c
===================================================================
--- scsi-reqfn-export.orig/drivers/scsi/scsi.c	2005-05-14 09:43:18.000000000 +0900
+++ scsi-reqfn-export/drivers/scsi/scsi.c	2005-05-14 09:45:59.000000000 +0900
@@ -638,10 +638,12 @@ int scsi_dispatch_cmd(struct scsi_cmnd *
 	}
 	spin_unlock_irqrestore(host->host_lock, flags);
 	if (rtn) {
-		atomic_inc(&cmd->device->iodone_cnt);
-		scsi_queue_insert(cmd,
-				(rtn == SCSI_MLQUEUE_DEVICE_BUSY) ?
-				 rtn : SCSI_MLQUEUE_HOST_BUSY);
+		if (scsi_delete_timer(cmd)) {
+			atomic_inc(&cmd->device->iodone_cnt);
+			scsi_queue_insert(cmd,
+					  (rtn == SCSI_MLQUEUE_DEVICE_BUSY) ?
+					  rtn : SCSI_MLQUEUE_HOST_BUSY);
+		}
 		SCSI_LOG_MLQUEUE(3,
 		    printk("queuecommand : request rejected\n"));
 	}
Index: scsi-reqfn-export/drivers/scsi/scsi_lib.c
===================================================================
--- scsi-reqfn-export.orig/drivers/scsi/scsi_lib.c	2005-05-14 09:43:18.000000000 +0900
+++ scsi-reqfn-export/drivers/scsi/scsi_lib.c	2005-05-14 09:45:59.000000000 +0900
@@ -128,13 +128,7 @@ int scsi_queue_insert(struct scsi_cmnd *
 		 printk("Inserting command %p into mlqueue\n", cmd));
 
 	/*
-	 * We are inserting the command into the ml queue.  First, we
-	 * cancel the timer, so it doesn't time out.
-	 */
-	scsi_delete_timer(cmd);
-
-	/*
-	 * Next, set the appropriate busy bit for the device/host.
+	 * Set the appropriate busy bit for the device/host.
 	 *
 	 * If the host/device isn't busy, assume that something actually
 	 * completed, and that we should be able to queue a command now.

Re: [PATCH scsi-misc-2.6 02/03] scsi: remove unnecessary scsi_delete_timer() call in scsi_reset_provider()

[Tejun Heo]

02_scsi_timer_remove_delete_timer_from_reset_provider.patch

	scsi_reset_provider() calls scsi_delete_timer() on exit which
	isn't necessary.  Remove it.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 scsi_error.c |    1 -
 1 files changed, 1 deletion(-)

Index: scsi-reqfn-export/drivers/scsi/scsi_error.c
===================================================================
--- scsi-reqfn-export.orig/drivers/scsi/scsi_error.c	2005-05-14 09:43:18.000000000 +0900
+++ scsi-reqfn-export/drivers/scsi/scsi_error.c	2005-05-14 09:45:59.000000000 +0900
@@ -1870,7 +1870,6 @@ scsi_reset_provider(struct scsi_device *
 		rtn = FAILED;
 	}
 
-	scsi_delete_timer(scmd);
 	scsi_next_command(scmd);
 	return rtn;
 }

Re: [PATCH scsi-misc-2.6 03/03] scsi: remove spurious if tests from scsi_eh_{times_out|done}

[Tejun Heo]

03_scsi_timer_eh_timer_remove_spurious_if.patch

	'if' tests which check if eh_action isn't NULL in both
	functions are always true.  Remove the redundant if's as it
	can give wrong impressions.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 scsi_error.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

Index: scsi-reqfn-export/drivers/scsi/scsi_error.c
===================================================================
--- scsi-reqfn-export.orig/drivers/scsi/scsi_error.c	2005-05-14 09:45:59.000000000 +0900
+++ scsi-reqfn-export/drivers/scsi/scsi_error.c	2005-05-14 09:45:59.000000000 +0900
@@ -434,8 +434,7 @@ static void scsi_eh_times_out(struct scs
 	SCSI_LOG_ERROR_RECOVERY(3, printk("%s: scmd:%p\n", __FUNCTION__,
 					  scmd));
 
-	if (scmd->device->host->eh_action)
-		up(scmd->device->host->eh_action);
+	up(scmd->device->host->eh_action);
 }
 
 /**
@@ -457,8 +456,7 @@ static void scsi_eh_done(struct scsi_cmn
 		SCSI_LOG_ERROR_RECOVERY(3, printk("%s scmd: %p result: %x\n",
 					   __FUNCTION__, scmd, scmd->result));
 
-		if (scmd->device->host->eh_action)
-			up(scmd->device->host->eh_action);
+		up(scmd->device->host->eh_action);
 	}
 }