[PATCH scsi-misc-2.6 00/04] scsi: scsi_request_fn() reimplementation

* [PATCH scsi-misc-2.6 00/04] scsi: scsi_request_fn() reimplementation
@ 2005-05-14 13:57 Tejun Heo
  2005-05-14 13:57 ` [PATCH scsi-misc-2.6 01/04] scsi: consolidate error handling out of scsi_init_io() into scsi_prep_fn() Tejun Heo
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Tejun Heo @ 2005-05-14 13:57 UTC (permalink / raw)
  To: James.Bottomley, axboe, Christoph Hellwig; +Cc: linux-scsi, linux-kernel

 Hello, James.

 This is repost of reqfn-reimplmentation patchset posted about a month
ago.  As REQ_SPECIAL semantics patchset, which this patchset depends
on, has been accepted with slight modification (REQ_SOFTBARRIER
handled by blk layer), this patchset is adjusted accordingly.  Also,
it's updated to use original scsi_add_timer function as timer API
change patch isn't accepted yet.

 scsi_reqfn_reimplementation patch depends on the timer update
patchset I've posted yesterday.  The only dependency is the removal of
scsi_dispatch_cmd() because the timer patchset modifies
scsi_queue_insert() call in that function.  If you decide to not
accept the timer updates, just removing scsi_cmd_get_serial() and
scsi_dispatch_cmd() by hand should suffice.

 Original description follows.

 This patchset reimplements scsi_request_fn().  All prep's are moved
into prep_fn and all state checking/issueing are moved into
scsi_reqfn.  prep_fn() only terminates/defers unpreparable requests
and all requests are terminated through scsi midlayer.

[ Start of patch descriptions ]

01_scsi_reqfn_consolidate_error_handling.patch
	: consolidate error handling out of scsi_init_io() into scsi_prep_fn()

	This patch fixes a queue stall bug which occurred when sgtable
	allocation failed and device_busy == 0.  When scsi_init_io()
	returns BLKPREP_DEFER or BLKPREP_KILL, it's supposed to free
	resources itself.  This patch consolidates defer and kill
	handling into scsi_prep_fn().

	Note that this patch doesn't consolidate state defer/kill
	handlings in scsi_prep_fn() as all state checks will be moved
	into scsi_reques_fn() by the following reqfn_reimpl patch.

	ret value checking was changed to switch() as in James's
	patch.  Also, kill: comment is copied from James's patch.

02_scsi_reqfn_move_preps_to_prep_fn.patch
	: move request preps in other places into prep_fn()

	Move request preparations scattered in scsi_request_fn() and
	scsi_dispatch_cmd() into scsi_prep_fn()

	* CDB_SIZE check in scsi_dispatch_cmd()
	* SCSI-2 LUN preparation in scsi_dispatch_cmd()

	No invalid request reaches scsi_request_fn() anymore.

	Note that scsi_init_cmd_errh() is still left in
	scsi_request_fn().  As all requeued requests need its sense
	buffer and result value cleared, we can't move this to
	prep_fn() yet.  This is moved into prep_fn in the following
	requeue path consoildation patchset.

03_scsi_reqfn_reimplementation.patch
	: reimplement scsi_request_fn()

	New scsi_request_fn() is formatted mostly as Chistoph Hellwig
	suggested.

	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.

04_scsi_reqfn_remove_wait_req_end_io.patch
	: remove unnecessary scsi_wait_req_end_io()

	As all requests are now terminated via scsi midlayer, we don't
	need to set end_io for special reqs, remove it.

[ End of patch descriptions ]

 Thanks a lot.

^ permalink raw reply	[flat|nested] 13+ messages in thread