[PATCH] scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

[PATCH] scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

[Gregory CLEMENT]

PREEMPT_RT turns spinlock in a mutex that cannot be used in interrupt
context. Since commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the
interrupt service routine to a threaded IRQ handler"), the hard
interrupt handler is not converted into a threaded interrupt handler
(due to the IRQF_ONESHOT flag). This can lead to the use of a sleeping
function inside the interrupt context.

[    0.654017] scsi host0: ufshcd
[    0.659867] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
[    0.659878] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/0
[    0.659882] preempt_count: 10001, expected: 0
[    0.659885] RCU nest depth: 0, expected: 0
[    0.659892] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.1.0-rc3 #1 PREEMPT_RT
[    0.659903] Call Trace:
[    0.659907] [<ffffffff800148fe>] dump_backtrace+0x1c/0x24
[    0.659924] [<ffffffff80001580>] show_stack+0x28/0x34
[    0.659931] [<ffffffff8000ea7c>] dump_stack_lvl+0x5e/0x86
[    0.659937] [<ffffffff8000eab8>] dump_stack+0x14/0x1c
[    0.659942] [<ffffffff8005d07e>] __might_resched+0x138/0x142
[    0.659956] [<ffffffff808bea82>] rt_spin_lock+0x38/0x144
[    0.659965] [<ffffffff806d4b0a>] ufshcd_sl_intr+0x30a/0x622
[    0.659973] [<ffffffff806d50bc>] ufshcd_intr+0x86/0x9c
[    0.659979] [<ffffffff8009f072>] __handle_irq_event_percpu+0xa6/0x370
[    0.659987] [<ffffffff8009f3d2>] handle_irq_event+0x42/0x98
[    0.659992] [<ffffffff800a43ce>] handle_fasteoi_irq+0x11a/0x244
[    0.660002] [<ffffffff8009e718>] handle_irq_desc+0x38/0x46
[    0.660007] [<ffffffff8009e764>] generic_handle_irq+0x26/0x2e
[    0.660013] [<ffffffff8054f1e2>] aplic_direct_handle_irq+0xa0/0x158
[    0.660022] [<ffffffff8009e718>] handle_irq_desc+0x38/0x46
[    0.660027] [<ffffffff8009e7f2>] generic_handle_domain_irq+0x12/0x1a
[    0.660032] [<ffffffff8054df10>] riscv_intc_irq+0x2a/0x64
[    0.660038] [<ffffffff808b2708>] handle_riscv_irq+0x52/0x82
[    0.660048] [<ffffffff808c2fca>] call_on_irq_stack+0x32/0x40

This commit mitigates the issue by directly registering the thread
interrupt handler without involving a hard IRQ handler. This will only
be done when PREEMP_RT is enabled, which automatically turns all
interrupt handlers into threaded interrupt handlers.

Fixes: 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service routine to a threaded IRQ handler")
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 drivers/ufs/core/ufshcd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index d3044a3089b53..6d82658a1a66b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -11235,9 +11235,17 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	 */
 	ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
 
-	/* IRQ registration */
+	/* IRQ registration
+	 * In the case of PREMMP_RT, directly use the threaded
+	 * interrupt to avoid using a spinlock (which could sleep)
+	 * in the hard IRQ handler.
+	 */
+#ifdef CONFIG_PREEMPT_RT
+	err = devm_request_irq(dev, irq, ufshcd_threaded_intr, IRQF_SHARED, UFSHCD, hba);
+#else
 	err = devm_request_threaded_irq(dev, irq, ufshcd_intr, ufshcd_threaded_intr,
 					IRQF_ONESHOT | IRQF_SHARED, UFSHCD, hba);
+#endif
 	if (err) {
 		dev_err(hba->dev, "request irq failed\n");
 		goto out_disable;

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260630-ufshcd-spinlock-sleep-fix-848811f28ad7

Best regards,
-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The static function `ufshcd_intr` becomes unused when `CONFIG_PREEMPT_RT` is enabled, causing a compiler warning.
- [Low] Typo in the newly added source code comment.
--

commit 6afdc0b243918ce89eb55cea6349f608516d9d03
Author: Gregory CLEMENT <gregory.clement@bootlin.com>

scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

This commit addresses a sleeping-in-hard-irq context bug under PREEMPT_RT. 
It mitigates the issue by bypassing the hard IRQ handler and directly 
registering the threaded interrupt handler when PREEMPT_RT is enabled.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index d3044a3089b53..6d82658a1a66b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -11235,9 +11235,17 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  	 */
>  	ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>  
> -	/* IRQ registration */
> +	/* IRQ registration
> +	 * In the case of PREMMP_RT, directly use the threaded

[Severity: Low]
This isn't a bug, but there is a typo in this comment ("PREMMP_RT" instead
of "PREEMPT_RT"). The commit subject also has a small typo ("PREEMP_RT").

> +	 * interrupt to avoid using a spinlock (which could sleep)
> +	 * in the hard IRQ handler.
> +	 */
> +#ifdef CONFIG_PREEMPT_RT
> +	err = devm_request_irq(dev, irq, ufshcd_threaded_intr, IRQF_SHARED, UFSHCD, hba);
> +#else
>  	err = devm_request_threaded_irq(dev, irq, ufshcd_intr, ufshcd_threaded_intr,
>  					IRQF_ONESHOT | IRQF_SHARED, UFSHCD, hba);
> +#endif

[Severity: Low]
Will this cause a compiler warning for an unused function when building with
CONFIG_PREEMPT_RT=y? 

Because the reference to ufshcd_intr() is compiled out in the #else block, 
but its static definition remains unconditionally compiled earlier in the 
file:

static irqreturn_t ufshcd_intr(int irq, void *__hba)
{
    ...
}

Could this break builds that have CONFIG_WERROR=y enabled due to the 
-Wunused-function warning?

>  	if (err) {
>  		dev_err(hba->dev, "request irq failed\n");
>  		goto out_disable;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-ufshcd-spinlock-sleep-fix-v1-1-339b05a1c6f4@bootlin.com?part=1

Re: [PATCH] scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

[Gregory CLEMENT]

Hello,

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The static function `ufshcd_intr` becomes unused when `CONFIG_PREEMPT_RT` is enabled, causing a compiler warning.
> - [Low] Typo in the newly added source code comment.

Both of Sachiko's comments are valid. I am addressing these issues in
the next version.

Gregory

> --
>
> commit 6afdc0b243918ce89eb55cea6349f608516d9d03
> Author: Gregory CLEMENT <gregory.clement@bootlin.com>
>
> scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.
>
> This commit addresses a sleeping-in-hard-irq context bug under PREEMPT_RT. 
> It mitigates the issue by bypassing the hard IRQ handler and directly 
> registering the threaded interrupt handler when PREEMPT_RT is enabled.
>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index d3044a3089b53..6d82658a1a66b 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -11235,9 +11235,17 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>>  	 */
>>  	ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>>  
>> -	/* IRQ registration */
>> +	/* IRQ registration
>> +	 * In the case of PREMMP_RT, directly use the threaded
>
> [Severity: Low]
> This isn't a bug, but there is a typo in this comment ("PREMMP_RT" instead
> of "PREEMPT_RT"). The commit subject also has a small typo ("PREEMP_RT").
>
>> +	 * interrupt to avoid using a spinlock (which could sleep)
>> +	 * in the hard IRQ handler.
>> +	 */
>> +#ifdef CONFIG_PREEMPT_RT
>> +	err = devm_request_irq(dev, irq, ufshcd_threaded_intr, IRQF_SHARED, UFSHCD, hba);
>> +#else
>>  	err = devm_request_threaded_irq(dev, irq, ufshcd_intr, ufshcd_threaded_intr,
>>  					IRQF_ONESHOT | IRQF_SHARED, UFSHCD, hba);
>> +#endif
>
> [Severity: Low]
> Will this cause a compiler warning for an unused function when building with
> CONFIG_PREEMPT_RT=y? 
>
> Because the reference to ufshcd_intr() is compiled out in the #else block, 
> but its static definition remains unconditionally compiled earlier in the 
> file:
>
> static irqreturn_t ufshcd_intr(int irq, void *__hba)
> {
>     ...
> }
>
> Could this break builds that have CONFIG_WERROR=y enabled due to the 
> -Wunused-function warning?
>
>>  	if (err) {
>>  		dev_err(hba->dev, "request irq failed\n");
>>  		goto out_disable;
>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260630-ufshcd-spinlock-sleep-fix-v1-1-339b05a1c6f4@bootlin.com?part=1

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] scsi: ufs: core: Avoid sleeping in hard interrupt context when PREEMP_RT is enabled.

[Sebastian Andrzej Siewior]

On 2026-06-30 11:55:23 [+0200], Gregory CLEMENT wrote:
> PREEMPT_RT turns spinlock in a mutex that cannot be used in interrupt

it is a spinlock_t and a sleeping lock. Also hard interrupt context not
interrupt.

> context. Since commit 3c7ac40d7322 ("scsi: ufs: core: Delegate the
> interrupt service routine to a threaded IRQ handler"), the hard
> interrupt handler is not converted into a threaded interrupt handler
> (due to the IRQF_ONESHOT flag). This can lead to the use of a sleeping
> function inside the interrupt context.
> 
…
> 
> This commit mitigates the issue by directly registering the thread
> interrupt handler without involving a hard IRQ handler. This will only
> be done when PREEMP_RT is enabled, which automatically turns all
> interrupt handlers into threaded interrupt handlers.

This sounds like threadirqs is still broken.

> Fixes: 3c7ac40d7322 ("scsi: ufs: core: Delegate the interrupt service routine to a threaded IRQ handler")
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  drivers/ufs/core/ufshcd.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index d3044a3089b53..6d82658a1a66b 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -11235,9 +11235,17 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  	 */
>  	ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
>  
> -	/* IRQ registration */
> +	/* IRQ registration
> +	 * In the case of PREMMP_RT, directly use the threaded
> +	 * interrupt to avoid using a spinlock (which could sleep)
> +	 * in the hard IRQ handler.
> +	 */
> +#ifdef CONFIG_PREEMPT_RT
> +	err = devm_request_irq(dev, irq, ufshcd_threaded_intr, IRQF_SHARED, UFSHCD, hba);
> +#else
>  	err = devm_request_threaded_irq(dev, irq, ufshcd_intr, ufshcd_threaded_intr,
>  					IRQF_ONESHOT | IRQF_SHARED, UFSHCD, hba);
> +#endif

No. No ifdefery this needs to be addressed properly.

From ufshcd_intr():
|         intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
|         enabled_intr_status = intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
|
|         ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS);

What does this do? Does it disable the IRQ source? If so then
IRQF_ONESHOT should be removed.

|         /* Directly handle interrupts since MCQ ESI handlers does the hard job */
|         return ufshcd_sl_intr(hba, enabled_intr_status);

If not, why is this not part of the threaded handler?

>  	if (err) {
>  		dev_err(hba->dev, "request irq failed\n");
>  		goto out_disable;
> 

Sebastian