mirrors/scst
1
0
Fork 0
mirror of https://github.com/SCST-project/scst.git synced 2026-08-17 04:36:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

scst_cmd_set_sn: remove lockless fast path

Browse Source 
The lockless fast path for scst_cmd_set_sn() can cause commands to
lockup in state EXEC_CHECK_SN when there are multiple scst_tgts
accessing the same scst_device, for example two initiators
connected to the two ports of a dual-port QLogic FC HBA in target mode
both reading from the same shared disk.  The multithreaded_init_done
value is too low-level for this; it does not take the higher-level
configuration into account.

- Remove the lockless fast path.
- Remove multithreaded_init_done, which enabled/disabled the lockless
  fast path.
- Push the locking down into scst_cmd_set_sn(), which will now apply
  regardless of set_sn_on_restart_cmd, which matters for mixed-driver
  (e.g. iSCSI+qla2xxx) target-mode setups.
- Remove a bunch of comments explaining the rules for the lockless fast
  path.

Fixes: https://github.com/SCST-project/scst/issues/333
Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
This commit is contained in:
Tony Battersby
2025-12-11 12:19:22 +03:00
committed by Gleb Chesnokov
parent cb6cdf3a82
commit 1a7cfc8e68
3 changed files with 12 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scst/include/scst.h
+1 -12
View File
@@ -755,12 +755,6 @@ struct scst_tgt_template {
/* True, if this target doesn't need "enabled" attribute */
unsigned enabled_attr_not_needed:1;
/*
* True, if this target adapter can call scst_cmd_init_done() from
* several threads at the same time.
*/
unsigned multithreaded_init_done:1;
/*
* True, if this target driver supports T10-PI (DIF), i.e. sending and
* receiving DIF PI tags. If false, SCST will not allow to add
@@ -2033,10 +2027,7 @@ struct scst_order_data {
atomic_t *cur_sn_slot;
atomic_t sn_slots[15];
/*
* Used to serialized scst_cmd_init_done() if the corresponding
* session's target template has multithreaded_init_done set
*/
/* Used to serialize scst_cmd_init_done(). */
spinlock_t init_done_lock;
};
@@ -3590,8 +3581,6 @@ void scst_cmd_init_done(struct scst_cmd *cmd, enum scst_exec_context pref_contex
* SCST done the command's preprocessing preprocessing_done() function
* should be called. The second argument sets preferred command execution
* context. See SCST_CONTEXT_* constants for details.
*
* See comment for scst_cmd_init_done() for the serialization requirements.
*/
static inline void scst_cmd_init_stage1_done(struct scst_cmd *cmd,
enum scst_exec_context pref_context, int set_sn)
scst/src/scst_targ.c
+11 -37
View File
@@ -783,20 +783,6 @@ out_redirect:
* initialization and also that the command is ready for execution. The
* second argument sets the preferred command execution context. See also
* SCST_CONTEXT_* constants for more information.
*
* !!IMPORTANT!!
*
* If cmd->set_sn_on_restart_cmd has not been set, this function, as well
* as scst_cmd_init_stage1_done() and scst_restart_cmd(), must not be
* called simultaneously for the same session (more precisely, for the same
* session/LUN, i.e. tgt_dev), i.e. they must be somehow externally
* serialized. This is needed to have lock free fast path in
* scst_cmd_set_sn(). For majority of targets those functions are naturally
* serialized by the single source of commands. Only some, like iSCSI
* immediate commands with multiple connections per session or scst_local,
* are exceptions. For it, some mutex/lock must be used for the
* serialization. Or, alternatively, multithreaded_init_done can be set in
* the target's template.
*/
void scst_cmd_init_done(struct scst_cmd *cmd, enum scst_exec_context pref_context)
{
@@ -1618,9 +1604,6 @@ static int scst_preprocessing_done(struct scst_cmd *cmd)
*
* The second argument sets completion status
* (see SCST_PREPROCESS_STATUS_* constants for details)
*
* See also comment for scst_cmd_init_done() for the serialization
* requirements.
*/
void scst_restart_cmd(struct scst_cmd *cmd, int status, enum scst_exec_context pref_context)
{
@@ -1652,10 +1635,8 @@ void scst_restart_cmd(struct scst_cmd *cmd, int status, enum scst_exec_context p
} else {
scst_set_cmd_state(cmd, SCST_CMD_STATE_TGT_PRE_EXEC);
}
if (cmd->set_sn_on_restart_cmd) {
EXTRACHECKS_BUG_ON(cmd->tgtt->multithreaded_init_done);
if (cmd->set_sn_on_restart_cmd)
scst_cmd_set_sn(cmd);
}
#ifdef CONFIG_SCST_TEST_IO_IN_SIRQ
if (cmd->op_flags & SCST_TEST_IO_IN_SIRQ_ALLOWED)
break;
@@ -3848,7 +3829,7 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)
EXTRACHECKS_BUG_ON(cmd->sn_set || cmd->hq_cmd_inced);
/* Optimized for lockless fast path of sequence of SIMPLE commands */
/* Optimized for fast path of sequence of SIMPLE commands */
scst_check_debug_sn(cmd);
@@ -3870,6 +3851,8 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)
}
}
spin_lock_irqsave(&order_data->init_done_lock, flags);
again:
switch (cmd->queue_type) {
case SCST_CMD_QUEUE_SIMPLE:
@@ -3927,7 +3910,7 @@ ordered:
} else {
order_data->prev_cmd_ordered = 1;
spin_lock_irqsave(&order_data->sn_lock, flags);
spin_lock(&order_data->sn_lock); /* irqs already off */
/*
* If no commands are going to reach
@@ -3948,7 +3931,7 @@ ordered:
scst_inc_expected_sn_idle(order_data);
}
}
spin_unlock_irqrestore(&order_data->sn_lock, flags);
spin_unlock(&order_data->sn_lock);
}
cmd->sn = order_data->curr_sn;
@@ -3957,9 +3940,9 @@ ordered:
case SCST_CMD_QUEUE_HEAD_OF_QUEUE:
TRACE_SN("HQ cmd %p (op %s)", cmd, scst_get_opcode_name(cmd));
spin_lock_irqsave(&order_data->sn_lock, flags);
spin_lock(&order_data->sn_lock); /* irqs already off */
order_data->hq_cmd_count++;
spin_unlock_irqrestore(&order_data->sn_lock, flags);
spin_unlock(&order_data->sn_lock);
cmd->hq_cmd_inced = 1;
goto out;
@@ -3977,6 +3960,7 @@ ordered:
order_data->cur_sn_slot - order_data->sn_slots);
out:
spin_unlock_irqrestore(&order_data->init_done_lock, flags);
TRACE_EXIT();
}
@@ -4226,18 +4210,8 @@ static int __scst_init_cmd(struct scst_cmd *cmd)
if (cmd->completed)
goto out;
if (!cmd->set_sn_on_restart_cmd) {
if (!cmd->tgtt->multithreaded_init_done) {
scst_cmd_set_sn(cmd);
} else {
struct scst_order_data *order_data = cmd->cur_order_data;
unsigned long flags;
spin_lock_irqsave(&order_data->init_done_lock, flags);
scst_cmd_set_sn(cmd);
spin_unlock_irqrestore(&order_data->init_done_lock, flags);
}
}
if (!cmd->set_sn_on_restart_cmd)
scst_cmd_set_sn(cmd);
} else if (res < 0) {
TRACE_DBG("Finishing cmd %p", cmd);
scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_lun_not_supported));
scst_local/scst_local.c
-1
View File
@@ -1352,7 +1352,6 @@ static struct scst_tgt_template scst_local_targ_tmpl = {
.name = "scst_local",
.sg_tablesize = 0xffff,
.xmit_response_atomic = 1,
.multithreaded_init_done = 1,
.enabled_attr_not_needed = 1,
.tgtt_attrs = scst_local_tgtt_attrs,
.tgt_attrs = scst_local_tgt_attrs,