On 9/29/24 11:45 PM, Peter Wang (王信友) wrote:
I'm not quite sure what you mean. Are you suggesting that scsi_done()
should not be called in the case of a SCSI Abort? This should be
unrelated to OCS: ABORTED (MCQ), because in the case of OCS: INVALID
(SDB), scsi_done() might also be called, and calling scsi_done()
should
not cause any issues. This is because it has already been aborted
by the SCSI timeout, so the test bit(SCMD_STATE_COMPLETE) would
directly return. Below is the call flow:
scsi_done
scsi_done_internal
if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
return;
Hi Peter,
The purpose of the SCMD_STATE_COMPLETE bit is to prevent that the SCSI
core tries to abort a SCSI command concurrently with the SCSI LLD (UFS
driver in this case) calling scsi_done(). Making scsi_done() calls no-
ops while an .eh_abort_handler() implementation is in progress is an
undocumented side effect of this mechanism. But since it is unlikely
that this behavior will change in the SCSI core, I'm OK with relying on
this behavior.
ufshcd_abort_one() does not set the SCMD_STATE_COMPLETE bit before it
tries to abort a SCSI command. I think this is wrong because plenty of
code in ufshcd_try_to_abort_task() relies on the assumption that
scsi_done() is not called while that function is in progress. Do you
agree that SCMD_STATE_COMPLETE should be set by ufshcd_abort_one()
before it calls ufshcd_try_to_abort_task()? If this change is made, a
consequence is that the completion handler won't inform the SCSI core
anymore that abortion of a command by ufshcd_abort_one() has completed.
Hence, the cmd->result value won't matter anymore for commands aborted
by ufshcd_abort_one() and how ufshcd_transfer_rsp_status() translates
OCS_ABORTED won't matter anymore either.
Thanks,
Bart.
