* [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; 3+ 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] 3+ 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
2026-06-19 2:30 ` Can Guo
0 siblings, 1 reply; 3+ 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] 3+ messages in thread* Re: [PATCH v2] scsi: ufs: core: Avoid possible memory reclaim deadlock in TX EQTR context
2026-06-18 14:21 ` sashiko-bot
@ 2026-06-19 2:30 ` Can Guo
0 siblings, 0 replies; 3+ messages in thread
From: Can Guo @ 2026-06-19 2:30 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
On 6/18/2026 10:21 PM, sashiko-bot@kernel.org wrote:
> 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?
In the initial patch series, error handling was not the key
consideration. Later on,
this has been considered and addressed. After proper (error injection)
test, the
fixes shall be submitted.
>
>> 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?
It depends on what is implemented in POST_CHANGE. But this is a good point,
I am considering to mandate the call to POST_CHANGE. If there is a
change for
this, the change will come along with the one mentioned above.
Thanks,
Can Guo.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-19 2:30 UTC | newest]
Thread overview: 3+ 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
2026-06-19 2:30 ` Can Guo
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.