lkml.org 
[lkml]   [2005]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH scsi-misc-2.6 05/08] scsi: remove a timer race from scsi_queue_insert() and cleanup timer
    Date
    05_scsi_timer_cleanup.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.

    While at it, as, once a scsi timer is added, it should expire
    or be deleted before reused, make scsi_add_timer() strict
    about timer reuses. Now timer expiration function clears
    ->function and all timer deletion should go through
    scsi_delete_timer(). Also, remove bogus ->eh_action tests
    from scsi_eh_{done|times_out} functions. The condition is
    always true and the test is somewhat misleading.

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

    aic7xxx/aic79xx_osm.c | 1 +
    aic7xxx/aic7xxx_osm.c | 1 +
    scsi.c | 7 ++++---
    scsi_error.c | 24 +++++++-----------------
    scsi_lib.c | 6 ------
    5 files changed, 13 insertions(+), 26 deletions(-)

    Index: scsi-export/drivers/scsi/aic7xxx/aic79xx_osm.c
    ===================================================================
    --- scsi-export.orig/drivers/scsi/aic7xxx/aic79xx_osm.c 2005-03-23 09:39:36.000000000 +0900
    +++ scsi-export/drivers/scsi/aic7xxx/aic79xx_osm.c 2005-03-23 09:40:10.000000000 +0900
    @@ -2725,6 +2725,7 @@ ahd_linux_dv_target(struct ahd_softc *ah
    /* Queue the command and wait for it to complete */
    /* Abuse eh_timeout in the scsi_cmnd struct for our purposes */
    init_timer(&cmd->eh_timeout);
    + cmd->eh_timeout.function = NULL;
    #ifdef AHD_DEBUG
    if ((ahd_debug & AHD_SHOW_MESSAGES) != 0)
    /*
    Index: scsi-export/drivers/scsi/aic7xxx/aic7xxx_osm.c
    ===================================================================
    --- scsi-export.orig/drivers/scsi/aic7xxx/aic7xxx_osm.c 2005-03-23 09:39:36.000000000 +0900
    +++ scsi-export/drivers/scsi/aic7xxx/aic7xxx_osm.c 2005-03-23 09:40:10.000000000 +0900
    @@ -2409,6 +2409,7 @@ ahc_linux_dv_target(struct ahc_softc *ah
    /* Queue the command and wait for it to complete */
    /* Abuse eh_timeout in the scsi_cmnd struct for our purposes */
    init_timer(&cmd->eh_timeout);
    + cmd->eh_timeout.function = NULL;
    #ifdef AHC_DEBUG
    if ((ahc_debug & AHC_SHOW_MESSAGES) != 0)
    /*
    Index: scsi-export/drivers/scsi/scsi.c
    ===================================================================
    --- scsi-export.orig/drivers/scsi/scsi.c 2005-03-23 09:40:09.000000000 +0900
    +++ scsi-export/drivers/scsi/scsi.c 2005-03-23 09:40:10.000000000 +0900
    @@ -639,9 +639,10 @@ 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))
    + 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-export/drivers/scsi/scsi_error.c
    ===================================================================
    --- scsi-export.orig/drivers/scsi/scsi_error.c 2005-03-23 09:40:10.000000000 +0900
    +++ scsi-export/drivers/scsi/scsi_error.c 2005-03-23 09:40:10.000000000 +0900
    @@ -107,14 +107,7 @@ int scsi_eh_scmd_add(struct scsi_cmnd *s
    void scsi_add_timer(struct scsi_cmnd *scmd, int timeout,
    void (*complete)(struct scsi_cmnd *))
    {
    -
    - /*
    - * If the clock was already running for this command, then
    - * first delete the timer. The timer handling code gets rather
    - * confused if we don't do this.
    - */
    - if (scmd->eh_timeout.function)
    - del_timer(&scmd->eh_timeout);
    + BUG_ON(scmd->eh_timeout.function);

    scmd->eh_timeout.data = (unsigned long)scmd;
    scmd->eh_timeout.expires = jiffies + timeout;
    @@ -170,6 +163,9 @@ void scsi_times_out(struct scsi_cmnd *sc
    {
    scsi_log_completion(scmd, TIMEOUT_ERROR);

    + scmd->eh_timeout.data = (unsigned long)NULL;
    + scmd->eh_timeout.function = NULL;
    +
    if (scmd->device->host->hostt->eh_timed_out)
    switch (scmd->device->host->hostt->eh_timed_out(scmd)) {
    case EH_HANDLED:
    @@ -442,9 +438,7 @@ static void scsi_eh_times_out(struct scs
    scsi_eh_eflags_set(scmd, SCSI_EH_REC_TIMEOUT);
    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);
    }

    /**
    @@ -459,15 +453,12 @@ static void scsi_eh_done(struct scsi_cmn
    * way of stopping the timeout handler from running, so we must
    * always defer to it.
    */
    - if (del_timer(&scmd->eh_timeout)) {
    + if (scsi_delete_timer(scmd)) {
    scmd->request->rq_status = RQ_SCSI_DONE;
    scmd->owner = SCSI_OWNER_ERROR_HANDLER;
    -
    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);
    }
    }

    @@ -1881,7 +1872,6 @@ scsi_reset_provider(struct scsi_device *
    rtn = FAILED;
    }

    - scsi_delete_timer(scmd);
    scsi_next_command(scmd);
    return rtn;
    }
    Index: scsi-export/drivers/scsi/scsi_lib.c
    ===================================================================
    --- scsi-export.orig/drivers/scsi/scsi_lib.c 2005-03-23 09:40:09.000000000 +0900
    +++ scsi-export/drivers/scsi/scsi_lib.c 2005-03-23 09:40:10.000000000 +0900
    @@ -229,12 +229,6 @@ 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.
    *
    * If the host/device isn't busy, assume that something actually
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-04-06 13:30    [W:2.986 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site