Re: [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The POST_CHANGE notification is skipped on error paths in ufshcd_tx_eqtr(), leaving variant-specific hardware state unbalanced.
- [High] Missing cleanup of TX Equalization settings and Power Mode on error paths in ufs_qcom_get_rx_fom().
--

commit 8d8593416ad1701fbb78203bd1a0efec4ccc4895
Author: Can Guo <can.guo@oss.qualcomm.com>

scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context
    
This patch uses memalloc_noio_save() and memalloc_noio_restore() in
ufshcd_tx_eqtr() to prevent potential memory reclaim deadlocks. TX EQTR
allocations in a quiesced UFS tagset context could trigger direct reclaim
depending on UFS I/O, leading to deadlocks.

Closes: https://sashiko.dev/#/patchset/20260615132834.2985346-1-can.guo@oss.qualcomm.com?part=2

>     Use memalloc_noio_save/restore() in ufshcd_tx_eqtr() to cover all
>     allocations in the TX EQTR call tree, including:
>     - params->eqtr_record in ufshcd_tx_eqtr()
>     - eqtr_data in __ufshcd_tx_eqtr()
>     - params in ufs_qcom_get_rx_fom()

[Severity: High]
This is a pre-existing issue, but does ufs_qcom_get_rx_fom() skip cleanup
on error paths?

The function overrides TX Equalization settings via
ufshcd_apply_tx_eq_settings() and then alters the power mode to test the FOM.

If ufshcd_change_power_mode() or ufs_qcom_host_sw_rx_fom() fails, it
returns the error directly:

drivers/ufs/host/ufs-qcom.c:ufs_qcom_get_rx_fom() {
    ...
    ret = ufs_qcom_host_sw_rx_fom(hba, pwr_mode->lane_rx, fom);
    if (ret) {
        dev_err(...);
        return ret;
    }
    ...
}

Does this direct return bypass the restoration of the original TX
Equalization settings, permanently leaking the test PHY equalization
settings into the hardware configuration?

> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index aa64f2bf4f1ef..d20c478659157 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
[ ... ]
> @@ -1248,6 +1260,9 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
>  	if (ret)
>  		ufshcd_tx_eqtr_unprepare(hba, &old_pwr_info);
>  
> +out_noio_restore:
> +	memalloc_noio_restore(noio_flag);
> +
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
ufshcd_tx_eqtr() skip restoring the hardware state?

When ufshcd_vops_tx_eqtr_notify() is called with PRE_CHANGE, it saves
critical PHY settings. If __ufshcd_tx_eqtr() fails, the code jumps to out
and skips the POST_CHANGE notification:

drivers/ufs/core/ufs-txeq.c:ufshcd_tx_eqtr() {
    ...
    ret = ufshcd_vops_tx_eqtr_notify(hba, PRE_CHANGE, pwr_mode);
    if (ret)
        goto out;

    ret = __ufshcd_tx_eqtr(hba, params, pwr_mode);
    if (ret)
        goto out;

    ret = ufshcd_vops_tx_eqtr_notify(hba, POST_CHANGE, pwr_mode);
    ...
}

Since POST_CHANGE is exclusively responsible for restoring the saved
hardware state, could skipping this permanently leave the PHY tuning
registers corrupted and prevent UFS link recovery?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618140941.902000-1-can.guo@oss.qualcomm.com?part=1