diff --git a/scst/README b/scst/README index 741fee880..26207bd2a 100644 --- a/scst/README +++ b/scst/README @@ -389,6 +389,14 @@ following entries: - handlers - this is a root subdirectory for all SCST dev handlers + - max_tasklet_cmd - specifies how many commands at max can be queued in + the SCST core simultaneously from all connected initiators to allow + processing commands in soft-IRQ context in tasklets. If the count of + the commands exceeds this value, then all of them will be processed + only in threads. This is to to prevent possible starvation under + heavy load and in some cases to improve performance by more evenly + spreading load over available CPUs. + - sgv - this is a root subdirectory for all SCST SGV caches - targets - this is a root subdirectory for all SCST targets diff --git a/scst/README_in-tree b/scst/README_in-tree index 312765c04..c80330695 100644 --- a/scst/README_in-tree +++ b/scst/README_in-tree @@ -257,6 +257,14 @@ following entries: - handlers - this is a root subdirectory for all SCST dev handlers + - max_tasklet_cmd - specifies how many commands at max can be queued in + the SCST core simultaneously from all connected initiators to allow + processing commands in soft-IRQ context in tasklets. If the count of + the commands exceeds this value, then all of them will be processed + only in threads. This is to to prevent possible starvation under + heavy load and in some cases to improve performance by more evenly + spreading load over available CPUs. + - sgv - this is a root subdirectory for all SCST SGV caches - targets - this is a root subdirectory for all SCST targets diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index 49e1d91dd..e9a572c87 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -127,6 +127,8 @@ struct kmem_cache *scst_cmd_cachep; unsigned long scst_trace_flag; #endif +int scst_max_tasklet_cmd = SCST_DEF_MAX_TASKLET_CMD; + unsigned long scst_flags; atomic_t scst_cmd_count; diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 34e12ad8e..c1b191f9e 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -169,6 +169,9 @@ extern struct scst_acg *scst_default_acg; extern unsigned int scst_setup_id; #endif +#define SCST_DEF_MAX_TASKLET_CMD 20 +extern int scst_max_tasklet_cmd; + extern spinlock_t scst_init_lock; extern struct list_head scst_init_cmd_list; extern wait_queue_head_t scst_init_cmd_list_waitQ; diff --git a/scst/src/scst_sysfs.c b/scst/src/scst_sysfs.c index 74891839c..369069eee 100644 --- a/scst/src/scst_sysfs.c +++ b/scst/src/scst_sysfs.c @@ -4458,6 +4458,49 @@ static struct kobj_attribute scst_setup_id_attr = __ATTR(setup_id, S_IRUGO | S_IWUSR, scst_setup_id_show, scst_setup_id_store); +static ssize_t scst_max_tasklet_cmd_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + int count; + + TRACE_ENTRY(); + + count = sprintf(buf, "%d\n%s\n", scst_max_tasklet_cmd, + (scst_max_tasklet_cmd == SCST_DEF_MAX_TASKLET_CMD) + ? "" : SCST_SYSFS_KEY_MARK); + + TRACE_EXIT(); + return count; +} + +static ssize_t scst_max_tasklet_cmd_store(struct kobject *kobj, + struct kobj_attribute *attr, const char *buf, size_t count) +{ + int res; + unsigned long val; + + TRACE_ENTRY(); + + res = strict_strtoul(buf, 0, &val); + if (res != 0) { + PRINT_ERROR("strict_strtoul() for %s failed: %d ", buf, res); + goto out; + } + + scst_max_tasklet_cmd = val; + PRINT_INFO("Changed scst_max_tasklet_cmd to %d", scst_max_tasklet_cmd); + + res = count; + +out: + TRACE_EXIT_RES(res); + return res; +} + +static struct kobj_attribute scst_max_tasklet_cmd_attr = + __ATTR(max_tasklet_cmd, S_IRUGO | S_IWUSR, scst_max_tasklet_cmd_show, + scst_max_tasklet_cmd_store); + #if defined(CONFIG_SCST_DEBUG) || defined(CONFIG_SCST_TRACING) static struct kobj_attribute scst_trace_level_attr = __ATTR(trace_level, S_IRUGO | S_IWUSR, scst_main_trace_level_show, @@ -4474,6 +4517,7 @@ static struct kobj_attribute scst_last_sysfs_mgmt_res_attr = static struct attribute *scst_sysfs_root_default_attrs[] = { &scst_threads_attr.attr, &scst_setup_id_attr.attr, + &scst_max_tasklet_cmd_attr.attr, #if defined(CONFIG_SCST_DEBUG) || defined(CONFIG_SCST_TRACING) &scst_trace_level_attr.attr, #endif diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 1467e05a4..782a5d75a 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -87,13 +87,24 @@ static inline void scst_schedule_tasklet(struct scst_cmd *cmd) struct scst_tasklet *t = &scst_tasklets[smp_processor_id()]; unsigned long flags; - spin_lock_irqsave(&t->tasklet_lock, flags); - TRACE_DBG("Adding cmd %p to tasklet %d cmd list", cmd, - smp_processor_id()); - list_add_tail(&cmd->cmd_list_entry, &t->tasklet_cmd_list); - spin_unlock_irqrestore(&t->tasklet_lock, flags); + if (atomic_read(&scst_cmd_count) <= scst_max_tasklet_cmd) { + spin_lock_irqsave(&t->tasklet_lock, flags); + TRACE_DBG("Adding cmd %p to tasklet %d cmd list", cmd, + smp_processor_id()); + list_add_tail(&cmd->cmd_list_entry, &t->tasklet_cmd_list); + spin_unlock_irqrestore(&t->tasklet_lock, flags); - tasklet_schedule(&t->tasklet); + tasklet_schedule(&t->tasklet); + } else { + spin_lock_irqsave(&cmd->cmd_threads->cmd_list_lock, flags); + TRACE_DBG("Too many tasklet commands (%d), adding cmd %p to " + "active cmd list", atomic_read(&scst_cmd_count), cmd); + list_add_tail(&cmd->cmd_list_entry, + &cmd->cmd_threads->active_cmd_list); + wake_up(&cmd->cmd_threads->cmd_list_waitQ); + spin_unlock_irqrestore(&cmd->cmd_threads->cmd_list_lock, flags); + } + return; } /** @@ -204,10 +215,14 @@ static int scst_init_cmd(struct scst_cmd *cmd, enum scst_exec_context *context) #endif /* Small context optimization */ - if (((*context == SCST_CONTEXT_TASKLET) || - (*context == SCST_CONTEXT_DIRECT_ATOMIC)) && - scst_cmd_is_expected_set(cmd)) { - if (cmd->expected_data_direction & SCST_DATA_WRITE) { + if ((*context == SCST_CONTEXT_TASKLET) || + (*context == SCST_CONTEXT_DIRECT_ATOMIC)) { + /* + * If any data_direction not set, it's SCST_DATA_UNKNOWN, + * which is 0, so we can safely | them + */ + BUILD_BUG_ON(SCST_DATA_UNKNOWN != 0); + if ((cmd->data_direction | cmd->expected_data_direction) & SCST_DATA_WRITE) { if (!test_bit(SCST_TGT_DEV_AFTER_INIT_WR_ATOMIC, &cmd->tgt_dev->tgt_dev_flags)) *context = SCST_CONTEXT_THREAD; @@ -297,6 +312,7 @@ void scst_cmd_init_done(struct scst_cmd *cmd, PRINT_ERROR("Wrong context %d in IRQ from target %s, use " "SCST_CONTEXT_THREAD instead", pref_context, cmd->tgtt->name); + dump_stack(); pref_context = SCST_CONTEXT_THREAD; } #endif @@ -371,14 +387,6 @@ active: scst_schedule_tasklet(cmd); break; - case SCST_CONTEXT_DIRECT: - scst_process_active_cmd(cmd, false); - break; - - case SCST_CONTEXT_DIRECT_ATOMIC: - scst_process_active_cmd(cmd, true); - break; - default: PRINT_ERROR("Context %x is undefined, using the thread one", pref_context); @@ -395,6 +403,14 @@ active: wake_up(&cmd->cmd_threads->cmd_list_waitQ); spin_unlock_irqrestore(&cmd->cmd_threads->cmd_list_lock, flags); break; + + case SCST_CONTEXT_DIRECT: + scst_process_active_cmd(cmd, false); + break; + + case SCST_CONTEXT_DIRECT_ATOMIC: + scst_process_active_cmd(cmd, true); + break; } out: @@ -508,7 +524,7 @@ static int scst_parse_cmd(struct scst_cmd *cmd) * It shouldn't be because of the SCST_TGT_DEV_AFTER_* * optimization. */ - TRACE_DBG("Dev handler %s parse() needs thread " + TRACE_MGMT_DBG("Dev handler %s parse() needs thread " "context, rescheduling", dev->handler->name); res = SCST_CMD_STATE_RES_NEED_THREAD; goto out; @@ -806,6 +822,21 @@ set_res: if (unlikely(cmd->completed)) goto out_done; +#ifndef CONFIG_SCST_TEST_IO_IN_SIRQ + /* + * We can't allow atomic command on the exec stages. It shouldn't + * be because of the SCST_TGT_DEV_AFTER_* optimization, but during + * parsing data_direction can change, so we need to recheck. + */ + if (unlikely(scst_cmd_atomic(cmd) && + !(cmd->data_direction & SCST_DATA_WRITE))) { + TRACE_DBG_FLAG(TRACE_DEBUG|TRACE_MINOR, "Atomic context and " + "non-WRITE data direction, rescheduling (cmd %p)", cmd); + res = SCST_CMD_STATE_RES_NEED_THREAD; + goto out; + } +#endif + out: TRACE_EXIT_HRES(res); return res; @@ -879,7 +910,7 @@ static int scst_prepare_space(struct scst_cmd *cmd) * It shouldn't be because of the SCST_TGT_DEV_AFTER_* * optimization. */ - TRACE_DBG("Dev handler %s alloc_data_buf() needs " + TRACE_MGMT_DBG("Dev handler %s alloc_data_buf() needs " "thread context, rescheduling", dev->handler->name); res = SCST_CMD_STATE_RES_NEED_THREAD; @@ -1069,6 +1100,7 @@ void scst_restart_cmd(struct scst_cmd *cmd, int status, PRINT_ERROR("Wrong context %d in IRQ from target %s, use " "SCST_CONTEXT_THREAD instead", pref_context, cmd->tgtt->name); + dump_stack(); pref_context = SCST_CONTEXT_THREAD; } #endif @@ -1095,6 +1127,7 @@ void scst_restart_cmd(struct scst_cmd *cmd, int status, case SCST_PREPROCESS_STATUS_ERROR_SENSE_SET: scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; case SCST_PREPROCESS_STATUS_ERROR_FATAL: @@ -1105,12 +1138,14 @@ void scst_restart_cmd(struct scst_cmd *cmd, int status, scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_hardw_error)); scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; default: PRINT_ERROR("%s() received unknown status %x", __func__, status); scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; } @@ -1135,7 +1170,15 @@ static int scst_rdy_to_xfer(struct scst_cmd *cmd) if ((tgtt->rdy_to_xfer == NULL) || unlikely(cmd->internal)) { cmd->state = SCST_CMD_STATE_TGT_PRE_EXEC; - res = SCST_CMD_STATE_RES_CONT_SAME; +#ifndef CONFIG_SCST_TEST_IO_IN_SIRQ + /* We can't allow atomic command on the exec stages */ + if (scst_cmd_atomic(cmd)) { + TRACE_DBG("NULL rdy_to_xfer() and atomic context, " + "rescheduling (cmd %p)", cmd); + res = SCST_CMD_STATE_RES_NEED_THREAD; + } else +#endif + res = SCST_CMD_STATE_RES_CONT_SAME; goto out; } @@ -1144,8 +1187,8 @@ static int scst_rdy_to_xfer(struct scst_cmd *cmd) * It shouldn't be because of the SCST_TGT_DEV_AFTER_* * optimization. */ - TRACE_DBG("Target driver %s rdy_to_xfer() needs thread " - "context, rescheduling", tgtt->name); + TRACE_MGMT_DBG("Target driver %s rdy_to_xfer() needs thread " + "context, rescheduling", tgtt->name); res = SCST_CMD_STATE_RES_NEED_THREAD; goto out; } @@ -1243,6 +1286,9 @@ static void scst_process_redirect_cmd(struct scst_cmd *cmd, TRACE_DBG("Context: %x", context); + if (check_retries) + scst_check_retries(tgt); + if (context == SCST_CONTEXT_SAME) context = scst_cmd_atomic(cmd) ? SCST_CONTEXT_DIRECT_ATOMIC : SCST_CONTEXT_DIRECT; @@ -1253,18 +1299,18 @@ static void scst_process_redirect_cmd(struct scst_cmd *cmd, break; case SCST_CONTEXT_DIRECT: - if (check_retries) - scst_check_retries(tgt); scst_process_active_cmd(cmd, false); break; + case SCST_CONTEXT_TASKLET: + scst_schedule_tasklet(cmd); + break; + default: PRINT_ERROR("Context %x is unknown, using the thread one", context); /* go through */ case SCST_CONTEXT_THREAD: - if (check_retries) - scst_check_retries(tgt); spin_lock_irqsave(&cmd->cmd_threads->cmd_list_lock, flags); TRACE_DBG("Adding cmd %p to active cmd list", cmd); if (unlikely(cmd->queue_type == SCST_CMD_QUEUE_HEAD_OF_QUEUE)) @@ -1276,12 +1322,6 @@ static void scst_process_redirect_cmd(struct scst_cmd *cmd, wake_up(&cmd->cmd_threads->cmd_list_waitQ); spin_unlock_irqrestore(&cmd->cmd_threads->cmd_list_lock, flags); break; - - case SCST_CONTEXT_TASKLET: - if (check_retries) - scst_check_retries(tgt); - scst_schedule_tasklet(cmd); - break; } TRACE_EXIT(); @@ -1320,6 +1360,7 @@ void scst_rx_data(struct scst_cmd *cmd, int status, PRINT_ERROR("Wrong context %d in IRQ from target %s, use " "SCST_CONTEXT_THREAD instead", pref_context, cmd->tgtt->name); + dump_stack(); pref_context = SCST_CONTEXT_THREAD; } #endif @@ -1367,6 +1408,7 @@ void scst_rx_data(struct scst_cmd *cmd, int status, case SCST_RX_STATUS_ERROR_SENSE_SET: scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; case SCST_RX_STATUS_ERROR_FATAL: @@ -1376,12 +1418,14 @@ void scst_rx_data(struct scst_cmd *cmd, int status, scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_hardw_error)); scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; default: PRINT_ERROR("scst_rx_data() received unknown status %x", status); scst_set_cmd_abnormal_done_state(cmd); + pref_context = SCST_CONTEXT_THREAD; break; } @@ -2601,7 +2645,7 @@ out_complete: if (ctx_changed) scst_reset_io_context(cmd->tgt_dev, old_ctx); - TRACE_EXIT(); + TRACE_EXIT_RES(res); return res; out_error: @@ -2633,7 +2677,6 @@ static inline int scst_real_exec(struct scst_cmd *cmd) __scst_cmd_get(cmd); res = scst_do_real_exec(cmd); - if (likely(res == SCST_EXEC_COMPLETED)) { scst_post_exec_sn(cmd, true); if (cmd->dev->scsi_dev != NULL) @@ -3030,8 +3073,14 @@ static int scst_pre_dev_done(struct scst_cmd *cmd) SCST_LOAD_SENSE(scst_sense_hardw_error)); } goto out; - } else if (unlikely(scst_check_sense(cmd))) + } else if (unlikely(scst_check_sense(cmd))) { + /* + * We can't allow atomic command on the exec stages, so + * restart to the thread + */ + res = SCST_CMD_STATE_RES_NEED_THREAD; goto out; + } if (likely(scsi_status_is_good(cmd->status))) { unsigned char type = cmd->dev->type; @@ -3275,8 +3324,8 @@ static int scst_dev_done(struct scst_cmd *cmd) * It shouldn't be because of the SCST_TGT_DEV_AFTER_* * optimization. */ - TRACE_DBG("Dev handler %s dev_done() needs thread " - "context, rescheduling", dev->handler->name); + TRACE_MGMT_DBG("Dev handler %s dev_done() needs thread " + "context, rescheduling", dev->handler->name); res = SCST_CMD_STATE_RES_NEED_THREAD; goto out; } @@ -3345,6 +3394,24 @@ static int scst_dev_done(struct scst_cmd *cmd) if (unlikely(cmd->internal)) cmd->state = SCST_CMD_STATE_FINISHED_INTERNAL; +#ifndef CONFIG_SCST_TEST_IO_IN_SIRQ + if (cmd->state != SCST_CMD_STATE_PRE_XMIT_RESP) { + /* We can't allow atomic command on the exec stages */ + if (scst_cmd_atomic(cmd)) { + switch (state) { + case SCST_CMD_STATE_TGT_PRE_EXEC: + case SCST_CMD_STATE_SEND_FOR_EXEC: + case SCST_CMD_STATE_LOCAL_EXEC: + case SCST_CMD_STATE_REAL_EXEC: + TRACE_DBG("Atomic context and redirect, " + "rescheduling (cmd %p)", cmd); + res = SCST_CMD_STATE_RES_NEED_THREAD; + break; + } + } + } +#endif + out: TRACE_EXIT_HRES(res); return res; @@ -3443,8 +3510,8 @@ static int scst_xmit_response(struct scst_cmd *cmd) * It shouldn't be because of the SCST_TGT_DEV_AFTER_* * optimization. */ - TRACE_DBG("Target driver %s xmit_response() needs thread " - "context, rescheduling", tgtt->name); + TRACE_MGMT_DBG("Target driver %s xmit_response() needs thread " + "context, rescheduling", tgtt->name); res = SCST_CMD_STATE_RES_NEED_THREAD; goto out; } @@ -3576,7 +3643,11 @@ void scst_tgt_cmd_done(struct scst_cmd *cmd, cmd->cmd_hw_pending = 0; + if (unlikely(cmd->tgt_dev == NULL)) + pref_context = SCST_CONTEXT_THREAD; + cmd->state = SCST_CMD_STATE_FINISHED; + scst_process_redirect_cmd(cmd, pref_context, 1); TRACE_EXIT(); @@ -4120,8 +4191,8 @@ void scst_process_active_cmd(struct scst_cmd *cmd, bool atomic) case SCST_CMD_STATE_PRE_DEV_DONE: res = scst_pre_dev_done(cmd); - EXTRACHECKS_BUG_ON(res == - SCST_CMD_STATE_RES_NEED_THREAD); + EXTRACHECKS_BUG_ON((res == SCST_CMD_STATE_RES_NEED_THREAD) && + (cmd->state == SCST_CMD_STATE_PRE_DEV_DONE)); break; case SCST_CMD_STATE_MODE_SELECT_CHECKS: