Re: [PATCH 1/2] scsi: ufs: core: Release command resources instead of force-completing

[Stanley Jhu <stanleyjhu@google.com>]

On 9/20/26 5:41 PM, Bart Van Assche wrote:
> No, this is not what I suggested.

Sorry about that - the Suggested-by: tag does not belong on this patch
and I will drop it.

> The text above says "implement this in three steps". Hence, this patch
> should have been three patches instead of one because of the "one change
> per patch" rule in the Linux kernel community.

Agreed.

> Why has ufshcd_clear_lu_cmds() been modified? This function is only
> called for pending commands so checking &hba->outstanding_reqs is not
> necessary.

> This is also wrong because this change will trigger a use-after-free in
> scmd_eh_abort_handler() if it decides to retry or finish a SCSI command.

Both hunks are out of scope for this patch and I will drop them.

> Did you read this code before you posted it? As its name suggests,
> ufshcd_release_scsi_cmd() is only called for SCSI commands. No device
> management command code should be added in this function.

You are right. In ufshcd_compl_one_cqe() I moved the
ufshcd_release_scsi_cmd() call out of the ufshcd_is_scsi_cmd() branch,
which let device management commands reach that function, and then
added an early return there to filter them back out. In the respin the
call will stay inside the branch, with no device management handling in
that function.

> Regarding (3), the UFS driver is a SCSI LLD (low-level driver) and hence
> should only do what is specific to the UFS driver. Completing commands
> after .eh_host_reset_handler() has been called is the responsibility of
> the SCSI core and should not be done by the UFS driver.

Agreed for the path where SCSI EH drove the reset. The case I am unsure
about is the other caller: ufshcd_err_handler() also runs from
hba->eh_work, scheduled by ufshcd_check_errors() on UIC and controller
errors. Those commands have not timed out and are not on
shost->eh_cmd_q, so SCSI EH never finishes them, and the handler leaves
them to the reset path on purpose:

	/*
	 * if host reset is required then skip clearing the pending
	 * transfers forcefully because they will get cleared during
	 * host reset and restore
	 */

Should those simply wait for the block layer timeout and come back
through SCSI EH?

Thanks,

Stanley