From 92b474595dbb7d808d985dc44f1f608649803d66 Mon Sep 17 00:00:00 2001 From: vlnb Date: Thu, 2 Nov 2006 10:54:10 +0000 Subject: [PATCH] - Fixes wrongly set context in scst_tgt_cmd_done() - Fixes retries for stateful devices git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@22 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scsi_tgt.h | 5 +- scst/src/dev_handlers/scst_changer.c | 2 + scst/src/dev_handlers/scst_disk.c | 4 +- scst/src/dev_handlers/scst_fileio.c | 2 +- scst/src/dev_handlers/scst_processor.c | 2 + scst/src/dev_handlers/scst_raid.c | 2 + scst/src/dev_handlers/scst_tape.c | 2 + scst/src/scst_lib.c | 2 +- scst/src/scst_priv.h | 14 ++- scst/src/scst_targ.c | 146 ++++++++++++------------- 10 files changed, 98 insertions(+), 83 deletions(-) diff --git a/scst/include/scsi_tgt.h b/scst/include/scsi_tgt.h index f13426b..5c39e7c 100644 --- a/scst/include/scsi_tgt.h +++ b/scst/include/scsi_tgt.h @@ -227,10 +227,9 @@ #define SCST_DEV_TM_NOT_COMPLETED 1 /************************************************************* - ** Default timeout and retries count for cmd's CDB execution - ** by SCSI mid-level (cmd's "timeout" and "retries" fields). + ** Default timeout for cmd's CDB execution + ** by SCSI mid-level (cmd's "timeout" field). *************************************************************/ -#define SCST_DEFAULT_RETRIES 5 #define SCST_DEFAULT_TIMEOUT (30*HZ) /************************************************************* diff --git a/scst/src/dev_handlers/scst_changer.c b/scst/src/dev_handlers/scst_changer.c index aa44976..c282575 100644 --- a/scst/src/dev_handlers/scst_changer.c +++ b/scst/src/dev_handlers/scst_changer.c @@ -141,6 +141,8 @@ int changer_parse(struct scst_cmd *cmd, const struct scst_info_cdb *info_cdb) * based on info_cdb, therefore change them only if necessary */ + cmd->retries = 1; + if (info_cdb->flags & SCST_LONG_TIMEOUT) { cmd->timeout = CHANGER_LONG_TIMEOUT; } else { diff --git a/scst/src/dev_handlers/scst_disk.c b/scst/src/dev_handlers/scst_disk.c index 63599fa..090ec10 100644 --- a/scst/src/dev_handlers/scst_disk.c +++ b/scst/src/dev_handlers/scst_disk.c @@ -58,7 +58,7 @@ exec: disk_exec, \ } -#define DISK_RETRIES 2 +#define DISK_RETRIES 5 #define DISK_SMALL_TIMEOUT (3 * HZ) #define DISK_REG_TIMEOUT (60 * HZ) #define DISK_LONG_TIMEOUT (3600 * HZ) @@ -294,6 +294,8 @@ int disk_parse(struct scst_cmd *cmd, const struct scst_info_cdb *info_cdb) * based on info_cdb, therefore change them only if necessary */ + cmd->retries = DISK_RETRIES; + if (info_cdb->flags & SCST_SMALL_TIMEOUT) { cmd->timeout = DISK_SMALL_TIMEOUT; } else if (info_cdb->flags & SCST_LONG_TIMEOUT) { diff --git a/scst/src/dev_handlers/scst_fileio.c b/scst/src/dev_handlers/scst_fileio.c index 58f7a04..f948c71 100644 --- a/scst/src/dev_handlers/scst_fileio.c +++ b/scst/src/dev_handlers/scst_fileio.c @@ -1725,7 +1725,7 @@ static void fileio_exec_read_capacity16(struct scst_cmd *cmd) nblocks = virt_dev->nblocks; memset(buffer, 0, sizeof(buffer)); - data64 = (uint64_t *)buffer; + data64 = (uint64_t*)buffer; data64[0] = cpu_to_be64(nblocks - 1); buffer[8] = (blocksize >> (BYTE * 3)) & 0xFF; buffer[9] = (blocksize >> (BYTE * 2)) & 0xFF; diff --git a/scst/src/dev_handlers/scst_processor.c b/scst/src/dev_handlers/scst_processor.c index c075dd8..3ff30bc 100644 --- a/scst/src/dev_handlers/scst_processor.c +++ b/scst/src/dev_handlers/scst_processor.c @@ -141,6 +141,8 @@ int processor_parse(struct scst_cmd *cmd, const struct scst_info_cdb *info_cdb) * based on info_cdb, therefore change them only if necessary */ + cmd->retries = 1; + if (info_cdb->flags & SCST_LONG_TIMEOUT) { cmd->timeout = PROCESSOR_LONG_TIMEOUT; } else { diff --git a/scst/src/dev_handlers/scst_raid.c b/scst/src/dev_handlers/scst_raid.c index 99abe32..1a1c941 100644 --- a/scst/src/dev_handlers/scst_raid.c +++ b/scst/src/dev_handlers/scst_raid.c @@ -141,6 +141,8 @@ int raid_parse(struct scst_cmd *cmd, const struct scst_info_cdb *info_cdb) * based on info_cdb, therefore change them only if necessary */ + cmd->retries = 1; + if (info_cdb->flags & SCST_LONG_TIMEOUT) { cmd->timeout = RAID_LONG_TIMEOUT; } else { diff --git a/scst/src/dev_handlers/scst_tape.c b/scst/src/dev_handlers/scst_tape.c index cc582f2..48ab98f 100644 --- a/scst/src/dev_handlers/scst_tape.c +++ b/scst/src/dev_handlers/scst_tape.c @@ -305,6 +305,8 @@ int tape_parse(struct scst_cmd *cmd, const struct scst_info_cdb *info_cdb) * based on info_cdb, therefore change them only if necessary */ + cmd->retries = 1; + if (info_cdb->flags & SCST_SMALL_TIMEOUT) { cmd->timeout = TAPE_SMALL_TIMEOUT; } else if (info_cdb->flags & SCST_LONG_TIMEOUT) { diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 39c4eb9..8bf9088 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -949,7 +949,7 @@ struct scst_cmd *scst_alloc_cmd(int gfp_mask) cmd->queue_type = SCST_CMD_QUEUE_UNTAGGED; cmd->timeout = SCST_DEFAULT_TIMEOUT; - cmd->retries = SCST_DEFAULT_RETRIES; + cmd->retries = 1; cmd->data_len = -1; cmd->tgt_resp_flags = SCST_TSC_FLAG_STATUS; cmd->resp_data_len = -1; diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index ce02fae..01b544d 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -102,9 +102,15 @@ #define SCST_CMD_MEM_TIMEOUT (120*HZ) static inline int scst_get_context(void) { - /* Be overinsured */ - return (in_atomic() || in_interrupt()) ? SCST_CONTEXT_DIRECT_ATOMIC : - SCST_CONTEXT_DIRECT; + if (in_irq()) + return SCST_CONTEXT_TASKLET; + if (in_softirq()) + return SCST_CONTEXT_DIRECT_ATOMIC; + if (in_interrupt()) /* Is it possible? */ + return SCST_CONTEXT_THREAD; + if (in_atomic()) + return SCST_CONTEXT_DIRECT_ATOMIC; + return SCST_CONTEXT_DIRECT; } #define SCST_MGMT_CMD_CACHE_STRING "scst_mgmt_cmd" @@ -250,6 +256,8 @@ static inline void scst_destroy_cmd(struct scst_cmd *cmd) return; } +void scst_proccess_redirect_cmd(struct scst_cmd *cmd, int context, + int check_retries); void scst_check_retries(struct scst_tgt *tgt, int processible_env); void scst_tgt_retry_timer_fn(unsigned long arg); diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 387e872..26e9ef0 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -744,10 +744,63 @@ out_dev_done: goto out; } -void scst_rx_data(struct scst_cmd *cmd, int status, int pref_context) +void scst_proccess_redirect_cmd(struct scst_cmd *cmd, int context, + int check_retries) { unsigned long flags; + int rc; + + TRACE_ENTRY(); + + TRACE_DBG("Context: %d", context); + + switch(context) { + case SCST_CONTEXT_DIRECT: + case SCST_CONTEXT_DIRECT_ATOMIC: + if (check_retries) + scst_check_retries(cmd->tgt, 0); + cmd->non_atomic_only = 0; + rc = __scst_process_active_cmd(cmd, context, 0); + if (rc == SCST_CMD_STATE_RES_NEED_THREAD) + goto out_thread; + break; + + default: + PRINT_ERROR_PR("Context %x is unknown, using the thread one", + context); + /* go through */ + case SCST_CONTEXT_THREAD: + if (check_retries) + scst_check_retries(cmd->tgt, 1); + goto out_thread; + + case SCST_CONTEXT_TASKLET: + if (check_retries) + scst_check_retries(cmd->tgt, 1); + cmd->non_atomic_only = 0; + spin_lock_irqsave(&scst_list_lock, flags); + TRACE_DBG("Moving cmd %p to active cmd list", cmd); + list_move_tail(&cmd->cmd_list_entry, &scst_active_cmd_list); + spin_unlock_irqrestore(&scst_list_lock, flags); + scst_schedule_tasklet(); + break; + } +out: + TRACE_EXIT(); + return; + +out_thread: + cmd->non_atomic_only = 1; + spin_lock_irqsave(&scst_list_lock, flags); + TRACE_DBG("Moving cmd %p to active cmd list", cmd); + list_move_tail(&cmd->cmd_list_entry, &scst_active_cmd_list); + spin_unlock_irqrestore(&scst_list_lock, flags); + wake_up(&scst_list_waitQ); + goto out; +} +void scst_rx_data(struct scst_cmd *cmd, int status, int pref_context) +{ TRACE_ENTRY(); TRACE_DBG("Preferred context: %d", pref_context); @@ -783,40 +836,12 @@ void scst_rx_data(struct scst_cmd *cmd, int status, int pref_context) default: PRINT_ERROR_PR("scst_rx_data() received unknown status %x", - status); + status); + cmd->state = SCST_CMD_STATE_DEV_DONE; break; } - switch (pref_context) { - case SCST_CONTEXT_DIRECT: - case SCST_CONTEXT_DIRECT_ATOMIC: - scst_check_retries(cmd->tgt, 0); - __scst_process_active_cmd(cmd, pref_context, 0); - break; - - default: - PRINT_ERROR_PR("Context %x is undefined, using thread one", - pref_context); - /* go through */ - case SCST_CONTEXT_THREAD: - spin_lock_irqsave(&scst_list_lock, flags); - TRACE_DBG("Moving cmd %p to active cmd list", cmd); - list_move_tail(&cmd->cmd_list_entry, &scst_active_cmd_list); - cmd->non_atomic_only = 1; - spin_unlock_irqrestore(&scst_list_lock, flags); - scst_check_retries(cmd->tgt, 1); - wake_up(&scst_list_waitQ); - break; - - case SCST_CONTEXT_TASKLET: - spin_lock_irqsave(&scst_list_lock, flags); - TRACE_DBG("Moving cmd %p to active cmd list", cmd); - list_move_tail(&cmd->cmd_list_entry, &scst_active_cmd_list); - spin_unlock_irqrestore(&scst_list_lock, flags); - scst_schedule_tasklet(); - scst_check_retries(cmd->tgt, 0); - break; - } + scst_proccess_redirect_cmd(cmd, pref_context, 1); TRACE_EXIT(); return; @@ -1088,7 +1113,7 @@ static void scst_cmd_done(struct scsi_cmnd *scsi_cmd) cmd->state = next_state; cmd->non_atomic_only = 0; - __scst_process_active_cmd(cmd, scst_get_context(), 0); + scst_proccess_redirect_cmd(cmd, scst_get_context(), 0); out: TRACE_EXIT(); @@ -1124,7 +1149,7 @@ static void scst_cmd_done(void *data, char *sense, int result, int resid) cmd->state = next_state; cmd->non_atomic_only = 0; - __scst_process_active_cmd(cmd, scst_get_context(), 0); + scst_proccess_redirect_cmd(cmd, scst_get_context(), 0); out: TRACE_EXIT(); @@ -1153,7 +1178,7 @@ static void scst_cmd_done_local(struct scst_cmd *cmd, int next_state) cmd->sg_cnt, sg, (void*)sg[0].page); for(i = 0; i < cmd->sg_cnt; ++i) { TRACE_BUFF_FLAG(TRACE_RECV_TOP, - "Exec'd sg:", page_address(sg[i].page), + "Exec'd sg", page_address(sg[i].page), sg[i].length); } } @@ -1184,7 +1209,7 @@ static void scst_cmd_done_local(struct scst_cmd *cmd, int next_state) cmd->state = next_state; cmd->non_atomic_only = 0; - __scst_process_active_cmd(cmd, scst_get_context(), 0); + scst_proccess_redirect_cmd(cmd, scst_get_context(), 0); TRACE_EXIT(); return; @@ -2175,7 +2200,7 @@ static int scst_xmit_response(struct scst_cmd *cmd) cmd->sg_cnt, sg, (void*)sg[0].page); for(i = 0; i < cmd->sg_cnt; ++i) { TRACE_BUFF_FLAG(TRACE_SEND_BOT, - "Xmitting sg:", page_address(sg[i].page), + "Xmitting sg", page_address(sg[i].page), sg[i].length); } } @@ -2277,47 +2302,12 @@ static int scst_finish_cmd(struct scst_cmd *cmd) void scst_tgt_cmd_done(struct scst_cmd *cmd) { - int res = 0; - unsigned long flags; - int context; - TRACE_ENTRY(); BUG_ON(cmd->state != SCST_CMD_STATE_XMIT_WAIT); - if (in_irq()) - context = SCST_CONTEXT_TASKLET; - else - context = scst_get_context(); - - TRACE_DBG("Context: %d", context); - cmd->non_atomic_only = 0; cmd->state = SCST_CMD_STATE_FINISHED; - - switch (context) { - case SCST_CONTEXT_DIRECT: - case SCST_CONTEXT_DIRECT_ATOMIC: - flags = 0; - scst_check_retries(cmd->tgt, 0); - res = __scst_process_active_cmd(cmd, context, 0); - BUG_ON(res == SCST_CMD_STATE_RES_NEED_THREAD); - break; - - case SCST_CONTEXT_TASKLET: - { - spin_lock_irqsave(&scst_list_lock, flags); - TRACE_DBG("Moving cmd %p to active cmd list", cmd); - list_move_tail(&cmd->cmd_list_entry, &scst_active_cmd_list); - spin_unlock_irqrestore(&scst_list_lock, flags); - scst_schedule_tasklet(); - scst_check_retries(cmd->tgt, 0); - break; - } - - default: - BUG(); - break; - } + scst_proccess_redirect_cmd(cmd, scst_get_context(), 1); TRACE_EXIT(); return; @@ -2577,6 +2567,14 @@ static void scst_do_job_active(struct list_head *active_cmd_list, int context) TRACE_ENTRY(); +#ifdef EXTRACHECKS + { + int c = (context & ~SCST_PROCESSIBLE_ENV); + WARN_ON((c != SCST_CONTEXT_DIRECT_ATOMIC) && + (c != SCST_CONTEXT_DIRECT)); + } +#endif + tm_dbg_check_released_cmds(); restart: @@ -2649,7 +2647,7 @@ int scst_cmd_thread(void *arg) scst_do_job_init(&scst_init_cmd_list); scst_do_job_active(&scst_active_cmd_list, - SCST_CONTEXT_THREAD|SCST_PROCESSIBLE_ENV); + SCST_CONTEXT_DIRECT|SCST_PROCESSIBLE_ENV); if (unlikely(test_bit(SCST_FLAG_SHUTDOWN, &scst_flags)) && list_empty(&scst_cmd_list) && -- 2.17.1