Re: [PATCH scsi-misc-2.6 03/04] scsi: reimplement scsi_request_fn()

[Tejun Heo <htejun@gmail.com>]

 Oops, I forgot to mention that reqfn is reformatted mostly as
suggested by Chritoph Hellwig.  Sorry.

On Tue, Apr 12, 2005 at 07:33:03PM +0900, Tejun Heo wrote:
> 03_scsi_reqfn_reimplementation.patch
> 
> 	This patch rewrites scsi_request_fn().	scsi_dispatch_cmd() is
> 	merged into scsi_request_fn().	Goals are
> 
> 	* Remove unnecessary operations (host_lock unlocking/locking,
> 	  recursing into scsi_run_queue(), ...)
> 	* Consolidate defer/kill paths.
> 	* Be concise.
> 
> 	The following bugs are fixed.
> 
> 	* All killed requests now get fully prep'ed and pass through
> 	  __scsi_done().  This is the only kill path.
> 		- scsi_cmnd leak in offline-kill path removed
> 		- unfinished request bug in
> 		  scsi_dispatch_cmd():SDEV_DEL-kill path removed.
> 		- commands are never terminated directly from blk
> 		  layer unless they are invalid, so no need to supply
> 		  req->end_io callback for special requests.
> 	* Timer is added while holding host_lock, after all conditions
> 	  are checked and serial number is assigned.  This guarantees
> 	  that until host_lock is released, the scsi_cmnd pointed to
> 	  by cmd isn't released.  That didn't hold in the original
> 	  code and, theoretically, the original code could access
> 	  already released cmd.
> 	* For the same reason, if shost->hostt->queuecommand() fails,
> 	  we try to delete the timer before releasing host_lock.
> 
> 	Other changes/notes
> 
> 	* host_lock is acquired and released only once.
> 	  enter (qlocked) -> enter loop -> dev-prep -> switch to hlock -\
> 			  ^---- switch to qlock <- issue <- host-prep <-/
> 	* unnecessary if () on get_device() removed.
> 	* loop on elv_next_request() instead of blk_queue_plugged().
> 	  We now explicitly break out of loop when we plug and check if
> 	  the queue has been plugged underneath us at the end of loop.
> 	* All device/host state checks are done in this function and
> 	  done only once while holding qlock/host_lock respectively.
> 	* Requests which get deferred during dev-prep are never
> 	  removed from request queue, so deferring is achieved by
> 	  simply breaking out of the loop and returning.
> 	* Failure of blk_queue_start_tag() on tagged queue is a BUG
> 	  now.	This condition should have been catched by
> 	  scsi_dev_queue_ready().
> 	* req->special == NULL test removed.  This just can't happen,
> 	  and even if it ever happens, scsi_request_fn() will
> 	  deterministically oops.
> 	* Requests which gets deferred during host-prep are requeued
> 	  using blk_requeue_request().	This is the only requeue path.
> 
> 	Note that scsi_kill_requests() still terminates requests using
> 	blk layer.  The path is circular-ref workaround and soon to be
> 	replaced, so ignore it for now.

-- 
tejun