Skip to content

Commit ae2fb3c

Browse files
Merge patch series "target: TMF and recovery fixes"
Mike Christie <michael.christie@oracle.com> says: The following patches apply over Martin's 6.4 branches and Linus's tree. They fix a couple regressions in iscsit that occur when there are TMRs executing and a connection is closed. It also includes Dimitry's fixes in related code paths for cmd cleanup when ERL2 is used and the write pending hang during conn cleanup. This version of the patchset brings it back to just regressions and fixes for bugs we have a lot of users hitting. I'm going to fix isert and get it hooked into iscsit properly in a second patchset, because this one was getting so large. I've also moved my cleanup type of patches for a 3rd patchset. Link: https://lore.kernel.org/r/20230319015620.96006-1-michael.christie@oracle.com Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2 parents 5c8c74e + ea87981 commit ae2fb3c

13 files changed

Lines changed: 230 additions & 118 deletions

File tree

drivers/infiniband/ulp/isert/ib_isert.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2506,8 +2506,8 @@ isert_wait4cmds(struct iscsit_conn *conn)
25062506
isert_info("iscsit_conn %p\n", conn);
25072507

25082508
if (conn->sess) {
2509-
target_stop_session(conn->sess->se_sess);
2510-
target_wait_for_sess_cmds(conn->sess->se_sess);
2509+
target_stop_cmd_counter(conn->cmd_cnt);
2510+
target_wait_for_cmds(conn->cmd_cnt);
25112511
}
25122512
}
25132513

drivers/target/iscsi/iscsi_target.c

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <target/target_core_base.h>
2727
#include <target/target_core_fabric.h>
2828

29+
#include <target/target_core_backend.h>
2930
#include <target/iscsi/iscsi_target_core.h>
3031
#include "iscsi_target_parameters.h"
3132
#include "iscsi_target_seq_pdu_list.h"
@@ -1190,9 +1191,10 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
11901191
* Initialize struct se_cmd descriptor from target_core_mod infrastructure
11911192
*/
11921193
__target_init_cmd(&cmd->se_cmd, &iscsi_ops,
1193-
conn->sess->se_sess, be32_to_cpu(hdr->data_length),
1194-
cmd->data_direction, sam_task_attr,
1195-
cmd->sense_buffer + 2, scsilun_to_int(&hdr->lun));
1194+
conn->sess->se_sess, be32_to_cpu(hdr->data_length),
1195+
cmd->data_direction, sam_task_attr,
1196+
cmd->sense_buffer + 2, scsilun_to_int(&hdr->lun),
1197+
conn->cmd_cnt);
11961198

11971199
pr_debug("Got SCSI Command, ITT: 0x%08x, CmdSN: 0x%08x,"
11981200
" ExpXferLen: %u, Length: %u, CID: %hu\n", hdr->itt,
@@ -2055,7 +2057,8 @@ iscsit_handle_task_mgt_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
20552057
__target_init_cmd(&cmd->se_cmd, &iscsi_ops,
20562058
conn->sess->se_sess, 0, DMA_NONE,
20572059
TCM_SIMPLE_TAG, cmd->sense_buffer + 2,
2058-
scsilun_to_int(&hdr->lun));
2060+
scsilun_to_int(&hdr->lun),
2061+
conn->cmd_cnt);
20592062

20602063
target_get_sess_cmd(&cmd->se_cmd, true);
20612064

@@ -4218,21 +4221,33 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
42184221
list_for_each_entry_safe(cmd, cmd_tmp, &tmp_list, i_conn_node) {
42194222
struct se_cmd *se_cmd = &cmd->se_cmd;
42204223

4221-
if (se_cmd->se_tfo != NULL) {
4222-
spin_lock_irq(&se_cmd->t_state_lock);
4223-
if (se_cmd->transport_state & CMD_T_ABORTED) {
4224+
if (!se_cmd->se_tfo)
4225+
continue;
4226+
4227+
spin_lock_irq(&se_cmd->t_state_lock);
4228+
if (se_cmd->transport_state & CMD_T_ABORTED) {
4229+
if (!(se_cmd->transport_state & CMD_T_TAS))
42244230
/*
42254231
* LIO's abort path owns the cleanup for this,
42264232
* so put it back on the list and let
42274233
* aborted_task handle it.
42284234
*/
42294235
list_move_tail(&cmd->i_conn_node,
42304236
&conn->conn_cmd_list);
4231-
} else {
4232-
se_cmd->transport_state |= CMD_T_FABRIC_STOP;
4233-
}
4237+
} else {
4238+
se_cmd->transport_state |= CMD_T_FABRIC_STOP;
4239+
}
4240+
4241+
if (cmd->se_cmd.t_state == TRANSPORT_WRITE_PENDING) {
4242+
/*
4243+
* We never submitted the cmd to LIO core, so we have
4244+
* to tell LIO to perform the completion process.
4245+
*/
42344246
spin_unlock_irq(&se_cmd->t_state_lock);
4247+
target_complete_cmd(&cmd->se_cmd, SAM_STAT_TASK_ABORTED);
4248+
continue;
42354249
}
4250+
spin_unlock_irq(&se_cmd->t_state_lock);
42364251
}
42374252
spin_unlock_bh(&conn->cmd_lock);
42384253

@@ -4243,6 +4258,16 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
42434258
iscsit_free_cmd(cmd, true);
42444259

42454260
}
4261+
4262+
/*
4263+
* Wait on commands that were cleaned up via the aborted_task path.
4264+
* LLDs that implement iscsit_wait_conn will already have waited for
4265+
* commands.
4266+
*/
4267+
if (!conn->conn_transport->iscsit_wait_conn) {
4268+
target_stop_cmd_counter(conn->cmd_cnt);
4269+
target_wait_for_cmds(conn->cmd_cnt);
4270+
}
42464271
}
42474272

42484273
static void iscsit_stop_timers_for_cmds(
@@ -4517,6 +4542,9 @@ int iscsit_close_session(struct iscsit_session *sess, bool can_sleep)
45174542
iscsit_stop_time2retain_timer(sess);
45184543
spin_unlock_bh(&se_tpg->session_lock);
45194544

4545+
if (sess->sess_ops->ErrorRecoveryLevel == 2)
4546+
iscsit_free_connection_recovery_entries(sess);
4547+
45204548
/*
45214549
* transport_deregister_session_configfs() will clear the
45224550
* struct se_node_acl->nacl_sess pointer now as a iscsi_np process context
@@ -4540,9 +4568,6 @@ int iscsit_close_session(struct iscsit_session *sess, bool can_sleep)
45404568

45414569
transport_deregister_session(sess->se_sess);
45424570

4543-
if (sess->sess_ops->ErrorRecoveryLevel == 2)
4544-
iscsit_free_connection_recovery_entries(sess);
4545-
45464571
iscsit_free_all_ooo_cmdsns(sess);
45474572

45484573
spin_lock_bh(&se_tpg->session_lock);

drivers/target/iscsi/iscsi_target_login.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,8 +1147,14 @@ static struct iscsit_conn *iscsit_alloc_conn(struct iscsi_np *np)
11471147
goto free_conn_cpumask;
11481148
}
11491149

1150+
conn->cmd_cnt = target_alloc_cmd_counter();
1151+
if (!conn->cmd_cnt)
1152+
goto free_conn_allowed_cpumask;
1153+
11501154
return conn;
11511155

1156+
free_conn_allowed_cpumask:
1157+
free_cpumask_var(conn->allowed_cpumask);
11521158
free_conn_cpumask:
11531159
free_cpumask_var(conn->conn_cpumask);
11541160
free_conn_ops:
@@ -1162,6 +1168,7 @@ static struct iscsit_conn *iscsit_alloc_conn(struct iscsi_np *np)
11621168

11631169
void iscsit_free_conn(struct iscsit_conn *conn)
11641170
{
1171+
target_free_cmd_counter(conn->cmd_cnt);
11651172
free_cpumask_var(conn->allowed_cpumask);
11661173
free_cpumask_var(conn->conn_cpumask);
11671174
kfree(conn->conn_ops);

drivers/target/target_core_device.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
741741
spin_lock_init(&dev->t10_alua.lba_map_lock);
742742

743743
INIT_WORK(&dev->delayed_cmd_work, target_do_delayed_work);
744+
mutex_init(&dev->lun_reset_mutex);
744745

745746
dev->t10_wwn.t10_dev = dev;
746747
/*

drivers/target/target_core_internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ int init_se_kmem_caches(void);
139139
void release_se_kmem_caches(void);
140140
u32 scsi_get_new_index(scsi_index_t);
141141
void transport_subsystem_check_init(void);
142-
void transport_uninit_session(struct se_session *);
143142
unsigned char *transport_dump_cmd_direction(struct se_cmd *);
144143
void transport_dump_dev_state(struct se_device *, char *, int *);
145144
void transport_dump_dev_info(struct se_device *, struct se_lun *,

drivers/target/target_core_tmr.c

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,23 @@ static void core_tmr_drain_tmr_list(
188188
* LUN_RESET tmr..
189189
*/
190190
spin_lock_irqsave(&dev->se_tmr_lock, flags);
191-
if (tmr)
192-
list_del_init(&tmr->tmr_list);
193191
list_for_each_entry_safe(tmr_p, tmr_pp, &dev->dev_tmr_list, tmr_list) {
192+
if (tmr_p == tmr)
193+
continue;
194+
194195
cmd = tmr_p->task_cmd;
195196
if (!cmd) {
196197
pr_err("Unable to locate struct se_cmd for TMR\n");
197198
continue;
198199
}
200+
201+
/*
202+
* We only execute one LUN_RESET at a time so we can't wait
203+
* on them below.
204+
*/
205+
if (tmr_p->function == TMR_LUN_RESET)
206+
continue;
207+
199208
/*
200209
* If this function was called with a valid pr_res_key
201210
* parameter (eg: for PROUT PREEMPT_AND_ABORT service action
@@ -379,14 +388,25 @@ int core_tmr_lun_reset(
379388
tmr_nacl->initiatorname);
380389
}
381390
}
391+
392+
393+
/*
394+
* We only allow one reset or preempt and abort to execute at a time
395+
* to prevent one call from claiming all the cmds causing a second
396+
* call from returning while cmds it should have waited on are still
397+
* running.
398+
*/
399+
mutex_lock(&dev->lun_reset_mutex);
400+
382401
pr_debug("LUN_RESET: %s starting for [%s], tas: %d\n",
383402
(preempt_and_abort_list) ? "Preempt" : "TMR",
384403
dev->transport->name, tas);
385-
386404
core_tmr_drain_tmr_list(dev, tmr, preempt_and_abort_list);
387405
core_tmr_drain_state_list(dev, prout_cmd, tmr_sess, tas,
388406
preempt_and_abort_list);
389407

408+
mutex_unlock(&dev->lun_reset_mutex);
409+
390410
/*
391411
* Clear any legacy SPC-2 reservation when called during
392412
* LOGICAL UNIT RESET

drivers/target/target_core_tpg.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ static void target_shutdown_sessions(struct se_node_acl *acl)
329329
restart:
330330
spin_lock_irqsave(&acl->nacl_sess_lock, flags);
331331
list_for_each_entry(sess, &acl->acl_sess_list, sess_acl_list) {
332-
if (atomic_read(&sess->stopped))
332+
if (sess->cmd_cnt && atomic_read(&sess->cmd_cnt->stopped))
333333
continue;
334334

335335
list_del_init(&sess->sess_acl_list);

0 commit comments

Comments
 (0)