Skip to content

Commit fa7e25c

Browse files
author
Nicholas Bellinger
committed
target: Fix unknown fabric callback queue-full errors
This patch fixes a set of queue-full response handling bugs, where outgoing responses are leaked when a fabric driver is propagating non -EAGAIN or -ENOMEM errors to target-core. It introduces TRANSPORT_COMPLETE_QF_ERR state used to signal when CHECK_CONDITION status should be generated, when fabric driver ->write_pending(), ->queue_data_in(), or ->queue_status() callbacks fail with non -EAGAIN or -ENOMEM errors, and data-transfer should not be retried. Note all fabric driver -EAGAIN and -ENOMEM errors are still retried indefinately with associated data-transfer callbacks, following existing queue-full logic. Also fix two missing ->queue_status() queue-full cases related to CMD_T_ABORTED w/ TAS status handling. Reported-by: Potnuri Bharat Teja <bharat@chelsio.com> Reviewed-by: Potnuri Bharat Teja <bharat@chelsio.com> Tested-by: Potnuri Bharat Teja <bharat@chelsio.com> Cc: Potnuri Bharat Teja <bharat@chelsio.com> Reported-by: Steve Wise <swise@opengridcomputing.com> Cc: Steve Wise <swise@opengridcomputing.com> Cc: Sagi Grimberg <sagi@grimberg.me> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
1 parent abe342a commit fa7e25c

File tree

2 files changed

+69
-34
lines changed

2 files changed

+69
-34
lines changed

‎drivers/target/target_core_transport.c

Lines changed: 68 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ struct kmem_cache *t10_alua_lba_map_cache;
6464
struct kmem_cache *t10_alua_lba_map_mem_cache;
6565

6666
static void transport_complete_task_attr(struct se_cmd *cmd);
67+
static int translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason);
6768
static void transport_handle_queue_full(struct se_cmd *cmd,
68-
struct se_device *dev);
69+
struct se_device *dev, int err, bool write_pending);
6970
static int transport_put_cmd(struct se_cmd *cmd);
7071
static void target_complete_ok_work(struct work_struct *work);
7172

@@ -804,7 +805,8 @@ void target_qf_do_work(struct work_struct *work)
804805

805806
if (cmd->t_state == TRANSPORT_COMPLETE_QF_WP)
806807
transport_write_pending_qf(cmd);
807-
else if (cmd->t_state == TRANSPORT_COMPLETE_QF_OK)
808+
else if (cmd->t_state == TRANSPORT_COMPLETE_QF_OK ||
809+
cmd->t_state == TRANSPORT_COMPLETE_QF_ERR)
808810
transport_complete_qf(cmd);
809811
}
810812
}
@@ -1719,7 +1721,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
17191721
}
17201722
trace_target_cmd_complete(cmd);
17211723
ret = cmd->se_tfo->queue_status(cmd);
1722-
if (ret == -EAGAIN || ret == -ENOMEM)
1724+
if (ret)
17231725
goto queue_full;
17241726
goto check_stop;
17251727
default:
@@ -1730,7 +1732,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
17301732
}
17311733

17321734
ret = transport_send_check_condition_and_sense(cmd, sense_reason, 0);
1733-
if (ret == -EAGAIN || ret == -ENOMEM)
1735+
if (ret)
17341736
goto queue_full;
17351737

17361738
check_stop:
@@ -1739,8 +1741,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
17391741
return;
17401742

17411743
queue_full:
1742-
cmd->t_state = TRANSPORT_COMPLETE_QF_OK;
1743-
transport_handle_queue_full(cmd, cmd->se_dev);
1744+
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
17441745
}
17451746
EXPORT_SYMBOL(transport_generic_request_failure);
17461747

@@ -1977,13 +1978,29 @@ static void transport_complete_qf(struct se_cmd *cmd)
19771978
int ret = 0;
19781979

19791980
transport_complete_task_attr(cmd);
1981+
/*
1982+
* If a fabric driver ->write_pending() or ->queue_data_in() callback
1983+
* has returned neither -ENOMEM or -EAGAIN, assume it's fatal and
1984+
* the same callbacks should not be retried. Return CHECK_CONDITION
1985+
* if a scsi_status is not already set.
1986+
*
1987+
* If a fabric driver ->queue_status() has returned non zero, always
1988+
* keep retrying no matter what..
1989+
*/
1990+
if (cmd->t_state == TRANSPORT_COMPLETE_QF_ERR) {
1991+
if (cmd->scsi_status)
1992+
goto queue_status;
19801993

1981-
if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) {
1982-
trace_target_cmd_complete(cmd);
1983-
ret = cmd->se_tfo->queue_status(cmd);
1984-
goto out;
1994+
cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE;
1995+
cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
1996+
cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER;
1997+
translate_sense_reason(cmd, TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE);
1998+
goto queue_status;
19851999
}
19862000

2001+
if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
2002+
goto queue_status;
2003+
19872004
switch (cmd->data_direction) {
19882005
case DMA_FROM_DEVICE:
19892006
if (cmd->scsi_status)
@@ -2007,19 +2024,33 @@ static void transport_complete_qf(struct se_cmd *cmd)
20072024
break;
20082025
}
20092026

2010-
out:
20112027
if (ret < 0) {
2012-
transport_handle_queue_full(cmd, cmd->se_dev);
2028+
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
20132029
return;
20142030
}
20152031
transport_lun_remove_cmd(cmd);
20162032
transport_cmd_check_stop_to_fabric(cmd);
20172033
}
20182034

2019-
static void transport_handle_queue_full(
2020-
struct se_cmd *cmd,
2021-
struct se_device *dev)
2035+
static void transport_handle_queue_full(struct se_cmd *cmd, struct se_device *dev,
2036+
int err, bool write_pending)
20222037
{
2038+
/*
2039+
* -EAGAIN or -ENOMEM signals retry of ->write_pending() and/or
2040+
* ->queue_data_in() callbacks from new process context.
2041+
*
2042+
* Otherwise for other errors, transport_complete_qf() will send
2043+
* CHECK_CONDITION via ->queue_status() instead of attempting to
2044+
* retry associated fabric driver data-transfer callbacks.
2045+
*/
2046+
if (err == -EAGAIN || err == -ENOMEM) {
2047+
cmd->t_state = (write_pending) ? TRANSPORT_COMPLETE_QF_WP :
2048+
TRANSPORT_COMPLETE_QF_OK;
2049+
} else {
2050+
pr_warn_ratelimited("Got unknown fabric queue status: %d\n", err);
2051+
cmd->t_state = TRANSPORT_COMPLETE_QF_ERR;
2052+
}
2053+
20232054
spin_lock_irq(&dev->qf_cmd_lock);
20242055
list_add_tail(&cmd->se_qf_node, &cmd->se_dev->qf_cmd_list);
20252056
atomic_inc_mb(&dev->dev_qf_count);
@@ -2083,7 +2114,7 @@ static void target_complete_ok_work(struct work_struct *work)
20832114
WARN_ON(!cmd->scsi_status);
20842115
ret = transport_send_check_condition_and_sense(
20852116
cmd, 0, 1);
2086-
if (ret == -EAGAIN || ret == -ENOMEM)
2117+
if (ret)
20872118
goto queue_full;
20882119

20892120
transport_lun_remove_cmd(cmd);
@@ -2109,7 +2140,7 @@ static void target_complete_ok_work(struct work_struct *work)
21092140
} else if (rc) {
21102141
ret = transport_send_check_condition_and_sense(cmd,
21112142
rc, 0);
2112-
if (ret == -EAGAIN || ret == -ENOMEM)
2143+
if (ret)
21132144
goto queue_full;
21142145

21152146
transport_lun_remove_cmd(cmd);
@@ -2134,7 +2165,7 @@ static void target_complete_ok_work(struct work_struct *work)
21342165
if (target_read_prot_action(cmd)) {
21352166
ret = transport_send_check_condition_and_sense(cmd,
21362167
cmd->pi_err, 0);
2137-
if (ret == -EAGAIN || ret == -ENOMEM)
2168+
if (ret)
21382169
goto queue_full;
21392170

21402171
transport_lun_remove_cmd(cmd);
@@ -2144,7 +2175,7 @@ static void target_complete_ok_work(struct work_struct *work)
21442175

21452176
trace_target_cmd_complete(cmd);
21462177
ret = cmd->se_tfo->queue_data_in(cmd);
2147-
if (ret == -EAGAIN || ret == -ENOMEM)
2178+
if (ret)
21482179
goto queue_full;
21492180
break;
21502181
case DMA_TO_DEVICE:
@@ -2157,7 +2188,7 @@ static void target_complete_ok_work(struct work_struct *work)
21572188
atomic_long_add(cmd->data_length,
21582189
&cmd->se_lun->lun_stats.tx_data_octets);
21592190
ret = cmd->se_tfo->queue_data_in(cmd);
2160-
if (ret == -EAGAIN || ret == -ENOMEM)
2191+
if (ret)
21612192
goto queue_full;
21622193
break;
21632194
}
@@ -2166,7 +2197,7 @@ static void target_complete_ok_work(struct work_struct *work)
21662197
queue_status:
21672198
trace_target_cmd_complete(cmd);
21682199
ret = cmd->se_tfo->queue_status(cmd);
2169-
if (ret == -EAGAIN || ret == -ENOMEM)
2200+
if (ret)
21702201
goto queue_full;
21712202
break;
21722203
default:
@@ -2180,8 +2211,8 @@ static void target_complete_ok_work(struct work_struct *work)
21802211
queue_full:
21812212
pr_debug("Handling complete_ok QUEUE_FULL: se_cmd: %p,"
21822213
" data_direction: %d\n", cmd, cmd->data_direction);
2183-
cmd->t_state = TRANSPORT_COMPLETE_QF_OK;
2184-
transport_handle_queue_full(cmd, cmd->se_dev);
2214+
2215+
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
21852216
}
21862217

21872218
void target_free_sgl(struct scatterlist *sgl, int nents)
@@ -2449,18 +2480,14 @@ transport_generic_new_cmd(struct se_cmd *cmd)
24492480
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
24502481

24512482
ret = cmd->se_tfo->write_pending(cmd);
2452-
if (ret == -EAGAIN || ret == -ENOMEM)
2483+
if (ret)
24532484
goto queue_full;
24542485

2455-
/* fabric drivers should only return -EAGAIN or -ENOMEM as error */
2456-
WARN_ON(ret);
2457-
2458-
return (!ret) ? 0 : TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
2486+
return 0;
24592487

24602488
queue_full:
24612489
pr_debug("Handling write_pending QUEUE__FULL: se_cmd: %p\n", cmd);
2462-
cmd->t_state = TRANSPORT_COMPLETE_QF_WP;
2463-
transport_handle_queue_full(cmd, cmd->se_dev);
2490+
transport_handle_queue_full(cmd, cmd->se_dev, ret, true);
24642491
return 0;
24652492
}
24662493
EXPORT_SYMBOL(transport_generic_new_cmd);
@@ -2470,10 +2497,10 @@ static void transport_write_pending_qf(struct se_cmd *cmd)
24702497
int ret;
24712498

24722499
ret = cmd->se_tfo->write_pending(cmd);
2473-
if (ret == -EAGAIN || ret == -ENOMEM) {
2500+
if (ret) {
24742501
pr_debug("Handling write_pending QUEUE__FULL: se_cmd: %p\n",
24752502
cmd);
2476-
transport_handle_queue_full(cmd, cmd->se_dev);
2503+
transport_handle_queue_full(cmd, cmd->se_dev, ret, true);
24772504
}
24782505
}
24792506

@@ -3011,6 +3038,8 @@ static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status)
30113038
__releases(&cmd->t_state_lock)
30123039
__acquires(&cmd->t_state_lock)
30133040
{
3041+
int ret;
3042+
30143043
assert_spin_locked(&cmd->t_state_lock);
30153044
WARN_ON_ONCE(!irqs_disabled());
30163045

@@ -3034,7 +3063,9 @@ static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status)
30343063
trace_target_cmd_complete(cmd);
30353064

30363065
spin_unlock_irq(&cmd->t_state_lock);
3037-
cmd->se_tfo->queue_status(cmd);
3066+
ret = cmd->se_tfo->queue_status(cmd);
3067+
if (ret)
3068+
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
30383069
spin_lock_irq(&cmd->t_state_lock);
30393070

30403071
return 1;
@@ -3055,6 +3086,7 @@ EXPORT_SYMBOL(transport_check_aborted_status);
30553086
void transport_send_task_abort(struct se_cmd *cmd)
30563087
{
30573088
unsigned long flags;
3089+
int ret;
30583090

30593091
spin_lock_irqsave(&cmd->t_state_lock, flags);
30603092
if (cmd->se_cmd_flags & (SCF_SENT_CHECK_CONDITION)) {
@@ -3090,7 +3122,9 @@ void transport_send_task_abort(struct se_cmd *cmd)
30903122
cmd->t_task_cdb[0], cmd->tag);
30913123

30923124
trace_target_cmd_complete(cmd);
3093-
cmd->se_tfo->queue_status(cmd);
3125+
ret = cmd->se_tfo->queue_status(cmd);
3126+
if (ret)
3127+
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
30943128
}
30953129

30963130
static void target_tmr_work(struct work_struct *work)

‎include/target/target_core_base.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ enum transport_state_table {
117117
TRANSPORT_ISTATE_PROCESSING = 11,
118118
TRANSPORT_COMPLETE_QF_WP = 18,
119119
TRANSPORT_COMPLETE_QF_OK = 19,
120+
TRANSPORT_COMPLETE_QF_ERR = 20,
120121
};
121122

122123
/* Used for struct se_cmd->se_cmd_flags */

0 commit comments

Comments
 (0)