[PATCH] Fix scsi_unblock_requests()

[PATCH] Fix scsi_unblock_requests()

[Justin T. Gibbs]

In its current implementation, scsi_unblock_requests() simply
clears host_self_blocked in the SCSI host struct.  This means
that either a transaction must complete or a new transaction
be issued before the mid-layer will recognize that it can
run the queues.  There is no guarantee that either of these
events will ever happen.

This patch attempts to run the queues manually when the host
unblocks.  scsi_queue_next_request() verifies all other state
to ensure queuing new transactions is safe prior to proceeding.

This patch is part of versions 6.1.10 and 6.1.11 of the
aic7xxx driver.  Its used, along with scsi_block_requests(), to
hold off the mid-layer during the initial bus settle delay without
resorting to a busy loop.

--
Justin

diff -u -r -N linux-2.4.3.virgin/drivers/scsi/scsi_lib.c linux-2.4.3/drivers/scsi/scsi_lib.c
--- linux-2.4.3.virgin/drivers/scsi/scsi_lib.c	Fri Mar  2 19:38:39 2001
+++ linux-2.4.3/drivers/scsi/scsi_lib.c	Thu Apr  5 14:28:17 2001
@@ -1108,9 +1108,13 @@
  */
 void scsi_unblock_requests(struct Scsi_Host * SHpnt)
 {
+	Scsi_Device *SDloop;
+
 	SHpnt->host_self_blocked = FALSE;
+	/* Now that we are unblocked, try to start the queues. */
+	for (SDloop = SHpnt->host_queue; SDloop; SDloop = SDloop->next)
+		scsi_queue_next_request(&SDloop->request_queue, NULL);
 }
-
 
 /*
  * Function:    scsi_report_bus_reset()

Re: [PATCH] Fix scsi_unblock_requests()

[Alan Cox]

> In its current implementation, scsi_unblock_requests() simply
> clears host_self_blocked in the SCSI host struct.  This means
> that either a transaction must complete or a new transaction

Suppose the queue is unblocked from inside the functions called to process
the request. In that situation the old code is correct and your code might
introduce other problems

> unblocks.  scsi_queue_next_request() verifies all other state
> to ensure queuing new transactions is safe prior to proceeding.

Including recursion ?

The patch seems right apart from checking these details out further. 

Re: [PATCH] Fix scsi_unblock_requests()

[Justin T. Gibbs]

>> In its current implementation, scsi_unblock_requests() simply
>> clears host_self_blocked in the SCSI host struct.  This means
>> that either a transaction must complete or a new transaction
>
>Suppose the queue is unblocked from inside the functions called to process
>the request. In that situation the old code is correct and your code might
>introduce other problems

Re-entrancy is only prevented by holding the io-request lock or in
some cases a per-controller or per-controller driver lock.
As the comment in scsi_unblock_requests() states, this API assumes
that no locks are held.  If you hold a lock in calling this routine,
you may deadlock.

>> unblocks.  scsi_queue_next_request() verifies all other state
>> to ensure queuing new transactions is safe prior to proceeding.
>
>Including recursion ?

I suppose a poorly implemented use of this API could tail recurse.
The aic7xxx driver calls this routine from a timeout handler so
there is no risk of stack explosion.

>The patch seems right apart from checking these details out further. 

Well, the patch was written to have minimal impact to an API that
has never been used.  A more correct solution might be to queue the
affected host controller to a queue drained by a bottom-half handler.

--
Justin