On 10/15/15, 12:12 PM, "target-devel-owner@xxxxxxxxxxxxxxx on behalf of
Bart Van Assche" <target-devel-owner@xxxxxxxxxxxxxxx on behalf of
bart.vanassche@xxxxxxxxxxx> wrote:
>On 10/14/2015 05:56 PM, Quinn Tran wrote:
>> QT> there are 2 places a command can reside: back end or front end.
>> This patch intercepts the command midway after the command has completed
>> from the backend (target_complete_cmd-> transport_handle_abort).
>>
>> IF the command is stuck at back end, then this TMR thread makes no
>> obvious call to abort the command because there is not an official API
>> call to the backend. It seems like the target_stop_cmd() call is
>> reserved for the backend folks to add their specific call to abort (I¹m
>> guessing based on the CMD_T_BUSY flag).
>>
>> IF the command is stuck at the front end, this patch removed the trigger
>> to cleanup the command or shortens command¹s life.
>>
>> Behind core_tmr_handle_tas_abort() is the
>> ³cmd->se_tfo->aborted_task(cmd)² call that is of interest that trigger
>> the cleanup.
>>
>> I think we should preserve the old semantic where we still keep the
>> aborted_task() call in this loop? Otherwise, half of the commands that
>> made to Fabric drivers don¹t get the trigger to cleanup.
>>
>> The kref_get_unless_zero() from above should prevent any race condition
>> or command premature free.
>
>Hello Quinn,
>
>Aborting a SCSI command that is being handled by the back-end driver
>would be an interesting feature. I think Hannes asked for that feature
>during the most recent LSF/MM workshop. However, as far as I know none
>of the existing LIO back-end drivers nor any of the SCST back-end
>drivers supports this feature today.
QT> This assumes that the back end is never at fault. God forbid if they
should fail. ;-).
>
>Regarding the aborted_task() call: had you noticed that that call has
>been moved to a new function, namely transport_handle_abort() ?
> Are you aware that the aborted_task() function must only be called after
>the
>back-end driver has finished processing a command ?
QT> Yes. I do see that you had move the call. This assume that all cmds
are at the back end and we catch them (mid way) as they are completed by
the back end and nudge each of them toward the abort path. Clean approach.
However, I¹m concern that the other commands that are already down at the
front end and stuck down there, when TMR request arrive. Meaning the
backend have completed the command and command is in status phase waiting
to be on the wire. By moving the aborted_task() call to
transport_handle_abort(), the patch remove the trigger from TMR thread to
abort commands that are pending in Fabric¹s layer, where the fabric layer
has dependency on today.
>My purpose was to
>keep the invocation of the aborted_task() callback function after the
>back-end driver has finished with its processing and before the target
>core cleans up a command. If you think that I missed a place where that
>callback function should be invoked please let me know.
QT> something like this. Piggy back on your fix.
core_tmr_drain_state_list()
{
while (!list_empty(&drain_task_list)) {
cmd = list_entry(drain_task_list.next, struct se_cmd, state_list);
list_del(&cmd->state_list);
----8<---
If ((cmd->t_state == TRANSPORT_COMPLETE) || (cmd->t_state ==
TRANSPORT_WRITE_PENDING) ||
(cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION)) {
// cmd is down at fabric layer. Send abort here because there is no other
callback.
If (cmd->t_state == TRANSPORT_WRITE_PENDING)
transport_cmd_check_stop_to_fabric();
cmd->se_tfo->aborted_task(cmd);
} else {
// cmd is at the back end. Aborted_task()/queue_status() will be sent at
transport_handle_abort()
target_stop_cmd();
}
---->8----
wait_for_completion(&cmd->finished);
target_put_sess_cmd(cmd);
}
}
>
>Bart.
>
>--
>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
Regards,
Quinn Tran
--
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
