All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context
@ 2026-06-18 14:09 Can Guo
  2026-06-18 14:21 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Can Guo @ 2026-06-18 14:09 UTC (permalink / raw)
  To: bvanassche, beanhuo, peter.wang, martin.petersen, mani
  Cc: linux-scsi, Can Guo, Alim Akhtar, Avri Altman,
	James E.J. Bottomley, open list

TX EQTR may run while devfreq gear scaling has quiesced the UFS tagset. In
that context, functions ufshcd_tx_eqtr(), __ufshcd_tx_eqtr() and
ufs_qcom_get_rx_fom() allocate memory with GFP_KERNEL. If direct reclaim
is triggered, reclaim/writeback can depend on I/O to UFS device. Because
the queue is quiesced, this can cause deadlock.

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()

This is preferred over tagging individual call sites with GFP_NOIO, as it
automatically covers any future allocations added anywhere in the call
tree without requiring each caller to be aware of this constraint.

Fixes: 03e5d38e2f98 ("scsi: ufs: core: Add support for TX Equalization")
Closes: https://sashiko.dev/#/patchset/20260615132834.2985346-1-can.guo@oss.qualcomm.com?part=2
Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
v1 -> v2:
- Replaced per-allocation GFP_NOIO usage with memalloc_noio_save/restore()
  around ufshcd_tx_eqtr() call tree.

 drivers/ufs/core/ufs-txeq.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 4b264adfdf49..9dca0cd344b8 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -10,6 +10,7 @@
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/kernel.h>
+#include <linux/sched/mm.h>
 #include <ufs/ufshcd.h>
 #include <ufs/unipro.h>
 #include "ufshcd-priv.h"
@@ -1212,14 +1213,25 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
 			  struct ufs_pa_layer_attr *pwr_mode)
 {
 	struct ufs_pa_layer_attr old_pwr_info;
+	unsigned int noio_flag;
 	int ret;
 
+	/*
+	 * ufshcd_tx_eqtr() is called from a power-mode-change context where
+	 * I/O is suspended. Use memalloc_noio_save() to propagate GFP_NOIO
+	 * to all allocations in the call tree instead of tagging each call
+	 * site individually.
+	 */
+	noio_flag = memalloc_noio_save();
+
 	if (!params->eqtr_record) {
 		params->eqtr_record = devm_kzalloc(hba->dev,
 						   sizeof(*params->eqtr_record),
 						   GFP_KERNEL);
-		if (!params->eqtr_record)
-			return -ENOMEM;
+		if (!params->eqtr_record) {
+			ret = -ENOMEM;
+			goto out_noio_restore;
+		}
 	}
 
 	memcpy(&old_pwr_info, &hba->pwr_info, sizeof(struct ufs_pa_layer_attr));
@@ -1244,6 +1256,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;
 }
 
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context
  2026-06-18 14:09 [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context Can Guo
@ 2026-06-18 14:21 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-18 14:21 UTC (permalink / raw)
  To: Can Guo; +Cc: linux-scsi

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-18 14:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 14:09 [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context Can Guo
2026-06-18 14:21 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.