Hi Bart,
I like this approach a lot. A few initial comments below:
> - list_move_tail(&tmr_p->tmr_list, &drain_tmr_list);
> + if (kref_get_unless_zero(&cmd->cmd_kref))
> + list_move_tail(&tmr_p->tmr_list, &drain_tmr_list);
Can you add a comment here why we might see a zero reference count, and
why it's safe to ignore the command in this case?
> @@ -282,8 +264,14 @@ static void core_tmr_drain_state_list(
> if (prout_cmd == cmd)
> continue;
>
> + if (!kref_get_unless_zero(&cmd->cmd_kref))
> + continue;
> +
Same here.
> list_move_tail(&cmd->state_list, &drain_task_list);
> cmd->state_active = false;
> + cmd->transport_state |= CMD_T_ABORTED;
> + if (tmr_nacl && tmr_nacl != cmd->se_sess->se_node_acl && tas)
> + cmd->send_abort_response = true;
And even if the old code didn't do this either, this condition is magic
enough to warrant a comment as well.
> +static void transport_handle_abort(struct se_cmd *cmd)
> +{
> + transport_lun_remove_cmd(cmd);
> +
> + if (cmd->send_abort_response) {
> + cmd->scsi_status = SAM_STAT_TASK_ABORTED;
> + pr_debug("Setting SAM_STAT_TASK_ABORTED status for CDB: 0x%02x, ITT: 0x%08llx\n",
> + cmd->t_task_cdb[0], cmd->tag);
> + trace_target_cmd_complete(cmd);
> + cmd->se_tfo->queue_status(cmd);
> + transport_cmd_check_stop_to_fabric(cmd);
> + } else {
> + /*
> + * Allow the fabric driver to unmap any resources before
> + * releasing the descriptor via TFO->release_cmd()
> + */
> + cmd->se_tfo->aborted_task(cmd);
> + /*
> + * To do: establish a unit attention condition on the I_T
> + * nexus associated with cmd. See also the paragraph "Aborting
> + * commands" in SAM.
> + */
> + if (transport_cmd_check_stop_to_fabric(cmd) == 0)
> + transport_put_cmd(cmd);
> + }
Why do we only have the call to transport_cmd_check_stop_to_fabric in
one of the branches here?
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
