Skip to content

scst_cmd_set_sn: remove lockless fast path #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lnocturno merged 1 commit into from
Dec 11, 2025
Merged

scst_cmd_set_sn: remove lockless fast path #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions scst/include/scst.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -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)
Expand Down
48 changes: 11 additions & 37 deletions scst/src/scst_targ.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,20 +783,6 @@ static int scst_init_cmd(struct scst_cmd *cmd, enum scst_exec_context *context)
* 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)
{
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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:
Expand Down Expand Up @@ -3927,7 +3910,7 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)
} 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
Expand All @@ -3948,7 +3931,7 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)
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;
Expand All @@ -3957,9 +3940,9 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)

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;

Expand All @@ -3977,6 +3960,7 @@ static void scst_cmd_set_sn(struct scst_cmd *cmd)
order_data->cur_sn_slot - order_data->sn_slots);

out:
spin_unlock_irqrestore(&order_data->init_done_lock, flags);
TRACE_EXIT();
}

Expand Down Expand Up @@ -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));
Expand Down
1 change: 0 additions & 1 deletion scst_local/scst_local.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading