On 9/18/24 6:29 AM, Peter Wang (王信友) wrote:
Basically, this patch currently only needs to handle requeueing
for the error handler abort.
The approach for DBR mode and MCQ mode should be consistent.
If receive an interrupt response (OCS:ABORTED or INVALID_OCS_VALUE),
then set DID_REQUEUE. If there is no interrupt, it will also set
SCSI DID_REQUEUE in ufshcd_err_handler through
ufshcd_complete_requests
with force_compl = true.
Reporting a completion for commands cleared by writing into the legacy
UTRLCLR register is not compliant with any version of the UFSHCI
standard. Reporting a completion for commands cleared by writing into
that register is problematic because it causes ufshcd_release_scsi_cmd()
to be called as follows:
ufshcd_sl_intr()
ufshcd_transfer_req_compl()
ufshcd_poll()
__ufshcd_transfer_req_compl()
ufshcd_compl_one_cqe()
cmd->result = ...
ufshcd_release_scsi_cmd()
scsi_done()
Calling ufshcd_release_scsi_cmd() if a command has been cleared is
problematic because the SCSI core does not expect this. If
ufshcd_try_to_abort_task() clears a SCSI command,
ufshcd_release_scsi_cmd() must not be called until the SCSI core
decides to release the command. This is why I wrote in a previous mail
that I think that a quirk should be introduced to suppress the
completions generated by clearing a SCSI command.
The more problematic part is with MCQ mode. To imitate the DBR
approach, we just need to set DID_REQUEUE upon receiving an interrupt.
Everything else remains the same. This would make things simpler.
Moving forward, if we want to simplify things and we have also
taken stock of the two or three scenarios where OCS: ABORTED occurs,
do we even need a flag? Couldn't we just set DID_REQUEUE directly
for OCS: ABORTED?
What do you think?
How about making ufshcd_compl_one_cqe() skip entries with status
OCS_ABORTED? That would make ufshcd_compl_one_cqe() behave as the
SCSI core expects, namely not freeing any command resources if a
SCSI command is aborted successfully.
This approach may require further changes to ufshcd_abort_all().
In that function there are separate code paths for legacy and MCQ
mode. This is less than ideal. Would it be possible to combine
these code paths by removing the ufshcd_complete_requests() call
from ufshcd_abort_all() and by handling completions from inside
ufshcd_abort_one()?
Thanks,
Bart.
