Skip to content

Commit

Permalink
target: Fix unknown fabric callback queue-full errors
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Nicholas Bellinger committed Mar 31, 2017
1 parent abe342a commit fa7e25c
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 34 deletions.
102 changes: 68 additions & 34 deletions drivers/target/target_core_transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ struct kmem_cache *t10_alua_lba_map_cache;
struct kmem_cache *t10_alua_lba_map_mem_cache;

static void transport_complete_task_attr(struct se_cmd *cmd);
static int translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason);
static void transport_handle_queue_full(struct se_cmd *cmd,
struct se_device *dev);
struct se_device *dev, int err, bool write_pending);
static int transport_put_cmd(struct se_cmd *cmd);
static void target_complete_ok_work(struct work_struct *work);

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

if (cmd->t_state == TRANSPORT_COMPLETE_QF_WP)
transport_write_pending_qf(cmd);
else if (cmd->t_state == TRANSPORT_COMPLETE_QF_OK)
else if (cmd->t_state == TRANSPORT_COMPLETE_QF_OK ||
cmd->t_state == TRANSPORT_COMPLETE_QF_ERR)
transport_complete_qf(cmd);
}
}
Expand Down Expand Up @@ -1719,7 +1721,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
}
trace_target_cmd_complete(cmd);
ret = cmd->se_tfo->queue_status(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;
goto check_stop;
default:
Expand All @@ -1730,7 +1732,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
}

ret = transport_send_check_condition_and_sense(cmd, sense_reason, 0);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;

check_stop:
Expand All @@ -1739,8 +1741,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
return;

queue_full:
cmd->t_state = TRANSPORT_COMPLETE_QF_OK;
transport_handle_queue_full(cmd, cmd->se_dev);
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
}
EXPORT_SYMBOL(transport_generic_request_failure);

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

transport_complete_task_attr(cmd);
/*
* If a fabric driver ->write_pending() or ->queue_data_in() callback
* has returned neither -ENOMEM or -EAGAIN, assume it's fatal and
* the same callbacks should not be retried. Return CHECK_CONDITION
* if a scsi_status is not already set.
*
* If a fabric driver ->queue_status() has returned non zero, always
* keep retrying no matter what..
*/
if (cmd->t_state == TRANSPORT_COMPLETE_QF_ERR) {
if (cmd->scsi_status)
goto queue_status;

if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) {
trace_target_cmd_complete(cmd);
ret = cmd->se_tfo->queue_status(cmd);
goto out;
cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE;
cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER;
translate_sense_reason(cmd, TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE);
goto queue_status;
}

if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
goto queue_status;

switch (cmd->data_direction) {
case DMA_FROM_DEVICE:
if (cmd->scsi_status)
Expand All @@ -2007,19 +2024,33 @@ static void transport_complete_qf(struct se_cmd *cmd)
break;
}

out:
if (ret < 0) {
transport_handle_queue_full(cmd, cmd->se_dev);
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
return;
}
transport_lun_remove_cmd(cmd);
transport_cmd_check_stop_to_fabric(cmd);
}

static void transport_handle_queue_full(
struct se_cmd *cmd,
struct se_device *dev)
static void transport_handle_queue_full(struct se_cmd *cmd, struct se_device *dev,
int err, bool write_pending)
{
/*
* -EAGAIN or -ENOMEM signals retry of ->write_pending() and/or
* ->queue_data_in() callbacks from new process context.
*
* Otherwise for other errors, transport_complete_qf() will send
* CHECK_CONDITION via ->queue_status() instead of attempting to
* retry associated fabric driver data-transfer callbacks.
*/
if (err == -EAGAIN || err == -ENOMEM) {
cmd->t_state = (write_pending) ? TRANSPORT_COMPLETE_QF_WP :
TRANSPORT_COMPLETE_QF_OK;
} else {
pr_warn_ratelimited("Got unknown fabric queue status: %d\n", err);
cmd->t_state = TRANSPORT_COMPLETE_QF_ERR;
}

spin_lock_irq(&dev->qf_cmd_lock);
list_add_tail(&cmd->se_qf_node, &cmd->se_dev->qf_cmd_list);
atomic_inc_mb(&dev->dev_qf_count);
Expand Down Expand Up @@ -2083,7 +2114,7 @@ static void target_complete_ok_work(struct work_struct *work)
WARN_ON(!cmd->scsi_status);
ret = transport_send_check_condition_and_sense(
cmd, 0, 1);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;

transport_lun_remove_cmd(cmd);
Expand All @@ -2109,7 +2140,7 @@ static void target_complete_ok_work(struct work_struct *work)
} else if (rc) {
ret = transport_send_check_condition_and_sense(cmd,
rc, 0);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;

transport_lun_remove_cmd(cmd);
Expand All @@ -2134,7 +2165,7 @@ static void target_complete_ok_work(struct work_struct *work)
if (target_read_prot_action(cmd)) {
ret = transport_send_check_condition_and_sense(cmd,
cmd->pi_err, 0);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;

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

trace_target_cmd_complete(cmd);
ret = cmd->se_tfo->queue_data_in(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;
break;
case DMA_TO_DEVICE:
Expand All @@ -2157,7 +2188,7 @@ static void target_complete_ok_work(struct work_struct *work)
atomic_long_add(cmd->data_length,
&cmd->se_lun->lun_stats.tx_data_octets);
ret = cmd->se_tfo->queue_data_in(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;
break;
}
Expand All @@ -2166,7 +2197,7 @@ static void target_complete_ok_work(struct work_struct *work)
queue_status:
trace_target_cmd_complete(cmd);
ret = cmd->se_tfo->queue_status(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;
break;
default:
Expand All @@ -2180,8 +2211,8 @@ static void target_complete_ok_work(struct work_struct *work)
queue_full:
pr_debug("Handling complete_ok QUEUE_FULL: se_cmd: %p,"
" data_direction: %d\n", cmd, cmd->data_direction);
cmd->t_state = TRANSPORT_COMPLETE_QF_OK;
transport_handle_queue_full(cmd, cmd->se_dev);

transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
}

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

ret = cmd->se_tfo->write_pending(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
if (ret)
goto queue_full;

/* fabric drivers should only return -EAGAIN or -ENOMEM as error */
WARN_ON(ret);

return (!ret) ? 0 : TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
return 0;

queue_full:
pr_debug("Handling write_pending QUEUE__FULL: se_cmd: %p\n", cmd);
cmd->t_state = TRANSPORT_COMPLETE_QF_WP;
transport_handle_queue_full(cmd, cmd->se_dev);
transport_handle_queue_full(cmd, cmd->se_dev, ret, true);
return 0;
}
EXPORT_SYMBOL(transport_generic_new_cmd);
Expand All @@ -2470,10 +2497,10 @@ static void transport_write_pending_qf(struct se_cmd *cmd)
int ret;

ret = cmd->se_tfo->write_pending(cmd);
if (ret == -EAGAIN || ret == -ENOMEM) {
if (ret) {
pr_debug("Handling write_pending QUEUE__FULL: se_cmd: %p\n",
cmd);
transport_handle_queue_full(cmd, cmd->se_dev);
transport_handle_queue_full(cmd, cmd->se_dev, ret, true);
}
}

Expand Down Expand Up @@ -3011,6 +3038,8 @@ static int __transport_check_aborted_status(struct se_cmd *cmd, int send_status)
__releases(&cmd->t_state_lock)
__acquires(&cmd->t_state_lock)
{
int ret;

assert_spin_locked(&cmd->t_state_lock);
WARN_ON_ONCE(!irqs_disabled());

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

spin_unlock_irq(&cmd->t_state_lock);
cmd->se_tfo->queue_status(cmd);
ret = cmd->se_tfo->queue_status(cmd);
if (ret)
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
spin_lock_irq(&cmd->t_state_lock);

return 1;
Expand All @@ -3055,6 +3086,7 @@ EXPORT_SYMBOL(transport_check_aborted_status);
void transport_send_task_abort(struct se_cmd *cmd)
{
unsigned long flags;
int ret;

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

trace_target_cmd_complete(cmd);
cmd->se_tfo->queue_status(cmd);
ret = cmd->se_tfo->queue_status(cmd);
if (ret)
transport_handle_queue_full(cmd, cmd->se_dev, ret, false);
}

static void target_tmr_work(struct work_struct *work)
Expand Down
1 change: 1 addition & 0 deletions include/target/target_core_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ enum transport_state_table {
TRANSPORT_ISTATE_PROCESSING = 11,
TRANSPORT_COMPLETE_QF_WP = 18,
TRANSPORT_COMPLETE_QF_OK = 19,
TRANSPORT_COMPLETE_QF_ERR = 20,
};

/* Used for struct se_cmd->se_cmd_flags */
Expand Down

0 comments on commit fa7e25c

Please sign in to comment.