On 11/16/2015 06:01 AM, Nicholas A. Bellinger wrote:
> void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
> {
> struct se_device *dev = cmd->se_dev;
> int success = scsi_status == GOOD;
> unsigned long flags;
>
>- cmd->scsi_status = scsi_status;
>+ if (cmd->transport_state & CMD_T_ABORTED) {
>+ transport_handle_abort(cmd);
>+ return;
>+ } else if (cmd->transport_state & (CMD_T_REQUEST_STOP | CMD_T_STOP)) {
>+ return;
>+ }
>
Backend drivers like IBLOCK can call bio_endio -> target_complete_cmd()
from interrupt context.
Invoking transport_handle_abort() -> TFO->queue_status() with any type
of real struct block_device based backend storage will immediately
explode. That is why there is a queue_work() right below this code to
invoke target_complete_ok_work() from process context.
Also, checking ->transport_state without ->t_state_lock in completion
path is incorrect.
Hi Nic,
Sorry but I do not agree that checking CMD_T_ABORTED without locking is
wrong. The worst that can happen if CMD_T_ABORTED is checked without
locking is that if this test happens within a few nanoseconds after the
CMD_T_ABORTED flag has been set that target_complete_cmd() proceeds with
executing the command instead of aborting it. This behavior is compliant
with the SCSI specs and would also occur if sending the ABORT command
from the initiator to the target is delayed slightly. The Linux kernel
contains many optimizations of this kind, namely not using locking to
check a flag when it is safe to do leave out locking.
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
