Re: [PATCH] scsi: ufs: mcq: Reap aborted CQEs before returning from abort

[Jeuk Kim]

Hi Bart,

Thanks for the review.

> If the late CQE arrives after the command has been resubmitted,
> ufshcd_compl_one_cqe() also overwrites cmd->result and calls scsi_done()
> for the new instance of the command

Fixed in v2.

> Please release the command only after the host reset has completed in
> the timeout case.

Fixed in v2. On timeout, abort returns FAILED without releasing DMA.
Recovery is left to the SCSI core.

> Note that ufshcd_mcq_force_compl_one() skips commands with
> SCMD_STATE_COMPLETE set

Fixed in v2. It now releases timed-out commands' resources without
changing their result or calling scsi_done().

> Have you considered setting a separate "completed" bit in
> ufshcd_compl_one_cqe() for every command type?

Done in v2. UFSHCD_LRB_COMPLETED covers all command types, and
ufshcd_release_scsi_cmd() retains its original semantics.

> Does UFSHCI guarantee that the OCS_ABORTED CQE has been posted by the
> time SQRTSy.CUS is set?

CUS alone does not establish that software has consumed the CQE.
v2 waits for driver completion in ufshcd_mcq_sq_cleanup(), covering
both abort and LU reset, and propagates LU cleanup failures.

> Please don't mix SCSI EH return codes with negative errno values in
> one return value.

Fixed in v2.

> Calling ufshcd_eh_host_reset_handler() from ufshcd_abort() is
> completely unacceptable.

Removed in v2.

> The kernel-doc for @flags should say which bits it holds
> (UFSHCD_LRB_*). I'd also define the bit next to struct ufshcd_lrb
> rather than in ufshcd-priv.h.

Fixed in v2.

> Please add an in-body From: Jeuk Kim <jeuk20.kim@xxxxxxxxxxx> line.
> Please add Cc: stable@xxxxxxxxxxxxxxx.

Added in v2.

Thanks,
Jeuk