10_scsi_request_fn_rewrite.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 changes fix bugs.
* 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.
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
scsi.c | 137 ------------------------------
scsi_lib.c | 277 +++++++++++++++++++++++++++++++++----------------------------
2 files changed, 153 insertions(+), 261 deletions(-)
Index: scsi-export/drivers/scsi/scsi.c
===================================================================
--- scsi-export.orig/drivers/scsi/scsi.c 2005-03-31 18:06:22.000000000
+0900
+++ scsi-export/drivers/scsi/scsi.c 2005-03-31 18:06:22.000000000 +0900
@@ -70,15 +70,6 @@
/*
- * Definitions and constants.
- */
-
-#define MIN_RESET_DELAY (2*HZ)
-
-/* Do not call reset on error if we just did a reset within 15 sec. */
-#define MIN_RESET_PERIOD (15*HZ)
-
-/*
* Note - the initial logging level can be set here to log events at boot time.
* After the system is up, you may enable logging via the /proc interface.
*/
@@ -495,134 +486,6 @@ void scsi_log_completion(struct scsi_cmn
}
#endif
-/*
- * Assign a serial number and pid to the request for error recovery
- * and debugging purposes. Protected by the Host_Lock of host.
- */
-static inline void scsi_cmd_get_serial(struct Scsi_Host *host, struct
scsi_cmnd *cmd)
-{
- cmd->serial_number = host->cmd_serial_number++;
- if (cmd->serial_number == 0)
- cmd->serial_number = host->cmd_serial_number++;
-
- cmd->pid = host->cmd_pid++;
- if (cmd->pid == 0)
- cmd->pid = host->cmd_pid++;
-}
-
-/*
- * Function: scsi_dispatch_command
- *
- * Purpose: Dispatch a command to the low-level driver.
- *
- * Arguments: cmd - command block we are dispatching.
- *
- * Notes:
- */
-int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
-{
- struct Scsi_Host *host = cmd->device->host;
- unsigned long flags = 0;
- unsigned long timeout;
- int rtn = 0;
-
- /* check if the device is still usable */
- if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
- /* in SDEV_DEL we error all commands. DID_NO_CONNECT
- * returns an immediate error upwards, and signals
- * that the device is no longer present */
- cmd->result = DID_NO_CONNECT << 16;
- atomic_inc(&cmd->device->iorequest_cnt);
- scsi_done(cmd);
- /* return 0 (because the command has been processed) */
- goto out;
- }
-
- /* Check to see if the scsi lld put this device into state SDEV_BLOCK.
*/
- if (unlikely(cmd->device->sdev_state == SDEV_BLOCK)) {
- /*
- * in SDEV_BLOCK, the command is just put back on the device
- * queue. The suspend state has already blocked the queue so
- * future requests should not occur until the device
- * transitions out of the suspend state.
- */
- scsi_queue_insert(cmd, SCSI_MLQUEUE_DEVICE_BUSY);
-
- SCSI_LOG_MLQUEUE(3, printk("queuecommand : device blocked \n"));
-
- /*
- * NOTE: rtn is still zero here because we don't need the
- * queue to be plugged on return (it's already stopped)
- */
- goto out;
- }
-
- /*
- * We will wait MIN_RESET_DELAY clock ticks after the last reset so
- * we can avoid the drive not being ready.
- */
- timeout = host->last_reset + MIN_RESET_DELAY;
-
- if (host->resetting && time_before(jiffies, timeout)) {
- int ticks_remaining = timeout - jiffies;
- /*
- * NOTE: This may be executed from within an interrupt
- * handler! This is bad, but for now, it'll do. The irq
- * level of the interrupt handler has been masked out by the
- * platform dependent interrupt handling code already, so the
- * sti() here will not cause another call to the SCSI host's
- * interrupt handler (assuming there is one irq-level per
- * host).
- */
- while (--ticks_remaining >= 0)
- mdelay(1 + 999 / HZ);
- host->resetting = 0;
- }
-
- /*
- * AK: unlikely race here: for some reason the timer could
- * expire before the serial number is set up below.
- */
- scsi_add_timer(cmd, cmd->timeout_per_command, scsi_times_out);
-
- scsi_log_send(cmd);
-
- /*
- * We will use a queued command if possible, otherwise we will
- * emulate the queuing and calling of completion function ourselves.
- */
-
- cmd->state = SCSI_STATE_QUEUED;
- cmd->owner = SCSI_OWNER_LOWLEVEL;
-
- atomic_inc(&cmd->device->iorequest_cnt);
-
- spin_lock_irqsave(host->host_lock, flags);
- scsi_cmd_get_serial(host, cmd);
-
- if (unlikely(test_bit(SHOST_CANCEL, &host->shost_state))) {
- cmd->result = (DID_NO_CONNECT << 16);
- scsi_done(cmd);
- } else {
- rtn = host->hostt->queuecommand(cmd, scsi_done);
- }
- spin_unlock_irqrestore(host->host_lock, flags);
- if (rtn) {
- 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"));
- }
-
- out:
- SCSI_LOG_MLQUEUE(3, printk("leaving scsi_dispatch_cmnd()\n"));
- return rtn;
-}
-
/*
* Function: scsi_init_cmd_from_req
*
Index: scsi-export/drivers/scsi/scsi_lib.c
===================================================================
--- scsi-export.orig/drivers/scsi/scsi_lib.c 2005-03-31 18:06:22.000000000
+0900
+++ scsi-export/drivers/scsi/scsi_lib.c 2005-03-31 18:06:22.000000000 +0900
@@ -28,6 +28,8 @@
#include "scsi_priv.h"
#include "scsi_logging.h"
+#define MIN_RESET_DELAY (2*HZ)
+
/*
* Macro to determine the size of SCSI command. This macro takes vendor
* unique commands into account. SCSI commands in groups 6 and 7 are
@@ -1023,32 +1025,6 @@ static int scsi_prep_fn(struct request_q
{
struct scsi_device *sdev = q->queuedata;
struct scsi_cmnd *cmd;
- int specials_only = 0;
-
- /*
- * Just check to see if the device is online. If it isn't, we
- * refuse to process any commands. The device must be brought
- * online before trying any recovery commands
- */
- if (unlikely(!scsi_device_online(sdev))) {
- printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to offline
device\n",
- sdev->host->host_no, sdev->id, sdev->lun);
- return BLKPREP_KILL;
- }
- if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
- /* OK, we're not in a running state don't prep
- * user commands */
- if (sdev->sdev_state == SDEV_DEL) {
- /* Device is fully deleted, no commands
- * at all allowed down */
- printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to dead
device\n",
- sdev->host->host_no, sdev->id, sdev->lun);
- return BLKPREP_KILL;
- }
- /* OK, we only allow special commands (i.e. not
- * user initiated ones */
- specials_only = sdev->sdev_state;
- }
/*
* The SPECIAL requests are things like character device or
@@ -1068,30 +1044,12 @@ static int scsi_prep_fn(struct request_q
} else
cmd = req->special;
} else if (req->flags & (REQ_CMD | REQ_BLOCK_PC)) {
-
- if(unlikely(specials_only)) {
- if(specials_only == SDEV_QUIESCE ||
- specials_only == SDEV_BLOCK)
- return BLKPREP_DEFER;
-
- printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to
device being removed\n",
- sdev->host->host_no, sdev->id, sdev->lun);
- return BLKPREP_KILL;
- }
-
-
- /*
- * Now try and find a command block that we can use.
- */
if (!req->special) {
cmd = scsi_get_command(sdev, GFP_ATOMIC);
if (unlikely(!cmd))
goto defer;
} else
cmd = req->special;
-
- /* pull a tag out of the request if we have one */
- cmd->tag = req->tag;
} else {
blk_dump_rq_flags(req, "SCSI bad req");
return BLKPREP_KILL;
@@ -1232,6 +1190,54 @@ static inline int scsi_host_queue_ready(
}
/*
+ * scsi_wait_reset: We will wait MIN_RESET_DELAY clock ticks after the
+ * last reset so we can avoid the drive not being ready.
+ *
+ * Called with no lock held and irq disabled.
+ */
+static inline void scsi_wait_reset(struct Scsi_Host *shost)
+{
+ unsigned long timeout = shost->last_reset + MIN_RESET_DELAY;
+
+ if (shost->resetting && time_before(jiffies, timeout)) {
+ int ticks_remaining = timeout - jiffies;
+ /*
+ * NOTE: This may be executed from within an interrupt
+ * handler! This is bad, but for now, it'll do. The
+ * irq level of the interrupt handler has been masked
+ * out by the platform dependent interrupt handling
+ * code already, so the local_irq_enable() here will
+ * not cause another call to the SCSI host's interrupt
+ * handler (assuming there is one irq-level per host).
+ */
+ local_irq_enable();
+
+ while (--ticks_remaining >= 0)
+ mdelay(1 + 999 / HZ);
+ shost->resetting = 0;
+
+ local_irq_disable();
+ }
+}
+
+/*
+ * scsi_cmd_get_serial: Assign a serial number and pid to the request
+ * for error recovery and debugging purposes.
+ *
+ * Called with host_lock held.
+ */
+static inline void scsi_cmd_get_serial(struct Scsi_Host *host, struct
scsi_cmnd *cmd)
+{
+ cmd->serial_number = host->cmd_serial_number++;
+ if (cmd->serial_number == 0)
+ cmd->serial_number = host->cmd_serial_number++;
+
+ cmd->pid = host->cmd_pid++;
+ if (cmd->pid == 0)
+ cmd->pid = host->cmd_pid++;
+}
+
+/*
* Function: scsi_request_fn()
*
* Purpose: Main strategy routine for SCSI.
@@ -1246,108 +1252,131 @@ static void scsi_request_fn(struct reque
{
struct scsi_device *sdev = q->queuedata;
struct Scsi_Host *shost = sdev->host;
- struct scsi_cmnd *cmd;
struct request *req;
+ struct scsi_cmnd *cmd;
+ int ret = 0;
- if(!get_device(&sdev->sdev_gendev))
- /* We must be tearing the block queue down already */
- return;
+ /* FIXME: Once fire & forgetters are fixed, this and the
+ * unlock_irq/put_device/lock_irq dance at the end of this
+ * function can go away. */
+ get_device(&sdev->sdev_gendev);
+
+ while ((req = elv_next_request(q))) {
+ enum scsi_device_state state;
+ int kill = 0, is_special = req->flags & REQ_SPECIAL;
- /*
- * To start with, we keep looping until the queue is empty, or until
- * the host is no longer able to accept any more requests.
- */
- while (!blk_queue_plugged(q)) {
- int rtn;
- /*
- * get next queueable request. We do this early to make sure
- * that the request is fully prepared even if we cannot
- * accept it.
- */
- req = elv_next_request(q);
- if (!req || !scsi_dev_queue_ready(q, sdev))
- break;
+ cmd = req->special;
+ state = cmd->device->sdev_state;
- if (unlikely(!scsi_device_online(sdev))) {
- printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to
offline device\n",
- sdev->host->host_no, sdev->id, sdev->lun);
- blkdev_dequeue_request(req);
- req->flags |= REQ_QUIET;
- while (end_that_request_first(req, 0, req->nr_sectors))
- ;
- end_that_request_last(req);
- continue;
+ if (scsi_device_online(sdev) && state != SDEV_DEL &&
+ (is_special || state != SDEV_CANCEL)) {
+ if (state == SDEV_BLOCK ||
+ (!is_special && state == SDEV_QUIESCE))
+ break;
+ if (!scsi_dev_queue_ready(q, sdev))
+ break;
+ } else {
+ printk(KERN_ERR
+ "scsi%d (%d:%d): rejecting I/O to %s\n",
+ shost->host_no, sdev->id, sdev->lun,
+ !scsi_device_online(sdev) ? "offline device" :
+ (state == SDEV_DEL ? "dead device"
+ : "device being removed"));
+ kill = 1;
}
-
- /*
- * Remove the request from the request list.
- */
- if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req)))
+ /* Start tag / remove from the request queue. */
+ if (blk_queue_tagged(q)) {
+ if (blk_queue_start_tag(q, req))
+ BUG();
+ cmd->tag = req->tag;
+ } else
blkdev_dequeue_request(req);
+
sdev->device_busy++;
+ /* Switch to host_lock. */
spin_unlock(q->queue_lock);
+ scsi_wait_reset(shost);
spin_lock(shost->host_lock);
- if (!scsi_host_queue_ready(q, shost, sdev))
- goto not_ready;
- if (sdev->single_lun) {
- if (scsi_target(sdev)->starget_sdev_user &&
- scsi_target(sdev)->starget_sdev_user != sdev)
- goto not_ready;
- scsi_target(sdev)->starget_sdev_user = sdev;
- }
- shost->host_busy++;
-
- /*
- * XXX(hch): This is rather suboptimal, scsi_dispatch_cmd will
- * take the lock again.
- */
- spin_unlock_irq(shost->host_lock);
+ if (!kill && !test_bit(SHOST_CANCEL, &shost->shost_state)) {
+ if (!scsi_host_queue_ready(q, shost, sdev))
+ goto requeue_out;
+ if (sdev->single_lun) {
+ struct scsi_target *target = scsi_target(sdev);
+ if (target->starget_sdev_user &&
+ target->starget_sdev_user != sdev)
+ goto requeue_out;
+ target->starget_sdev_user = sdev;
+ }
- cmd = req->special;
- if (unlikely(cmd == NULL)) {
- printk(KERN_CRIT "impossible request in %s.\n"
- "please mail a stack trace to "
- "linux-scsi@vger.kernel.org",
- __FUNCTION__);
- BUG();
+ shost->host_busy++;
+ scsi_log_send(cmd);
+ scsi_cmd_get_serial(shost, cmd);
+ scsi_add_timer(cmd, cmd->timeout_per_command,
+ scsi_times_out);
+
+ cmd->state = SCSI_STATE_QUEUED;
+ cmd->owner = SCSI_OWNER_LOWLEVEL;
+
+ ret = shost->hostt->queuecommand(cmd, scsi_done);
+ if (ret == 0)
+ atomic_inc(&sdev->iorequest_cnt);
+ else {
+ SCSI_LOG_MLQUEUE(3,
+ printk("%s: queuecommand deferred request
(%d)\n",
+ __FUNCTION__, ret));
+ /* Timer should be deleted while holding
+ * the host_lock. Once it gets released, we
+ * don't know if cmd is still there or not. */
+ if (scsi_delete_timer(cmd)) {
+ shost->host_busy--;
+ goto block_requeue_out;
+ }
+ }
+ } else {
+ SCSI_LOG_MLQUEUE(3,
+ printk("%s: rejecting request\n", __FUNCTION__));
+ shost->host_busy++;
+ atomic_inc(&sdev->iorequest_cnt);
+ cmd->result = DID_NO_CONNECT << 16;
+ __scsi_done(cmd);
}
- /*
- * Dispatch the command to the low-level driver.
- */
- rtn = scsi_dispatch_cmd(cmd);
- spin_lock_irq(q->queue_lock);
- if(rtn) {
- /* we're refusing the command; because of
- * the way locks get dropped, we need to
- * check here if plugging is required */
- if(sdev->device_busy == 0)
- blk_plug_device(q);
+ /* Switch back to queue_lock. */
+ spin_unlock(shost->host_lock);
+ spin_lock(q->queue_lock);
+ /* queuecommand failed and timer took the request, bail out. */
+ if (ret)
break;
- }
- }
+ /* The queue could have been plugged underneath us. */
+ if (blk_queue_plugged(q))
+ break;
+ }
goto out;
- not_ready:
- spin_unlock_irq(shost->host_lock);
+ block_requeue_out:
+ if (ret == SCSI_MLQUEUE_DEVICE_BUSY)
+ sdev->device_blocked = sdev->max_device_blocked;
+ else
+ shost->host_blocked = shost->max_host_blocked;
+ requeue_out:
+ /* Switch back to queue_lock */
+ spin_unlock(shost->host_lock);
+ spin_lock(q->queue_lock);
- /*
- * lock q, handle tag, requeue req, and decrement device_busy. We
- * must return with queue_lock held.
- *
- * Decrementing device_busy without checking it is OK, as all such
- * cases (host limits or settings) should run the queue at some
- * later time.
- */
- spin_lock_irq(q->queue_lock);
+ cmd->state = SCSI_STATE_MLQUEUE;
+ cmd->owner = SCSI_OWNER_MIDLEVEL;
+
+ SCSI_LOG_MLQUEUE(3,
+ printk("%s: requeueing request\n", __FUNCTION__));
+
+ req->flags |= REQ_SOFTBARRIER; /* Don't pass this request. */
blk_requeue_request(q, req);
- sdev->device_busy--;
- if(sdev->device_busy == 0)
+ if (--sdev->device_busy == 0)
blk_plug_device(q);
out:
/* must be careful here...if we trigger the ->remove() function
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
