[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

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

[Bart Van Assche]

On 6/30/26 7:15 AM, Sebastian Andrzej Siewior wrote:
>  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.

Writing into the REG_INTERRUPT_STATUS register clears the bits that are
set in the first argument of writel().

> |         /* 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 MCQ is disabled, ufshcd_sl_intr() processes all types of interrupts.
If MCQ is enabled, the ESI interrupt handlers process I/O completions
and ufshcd_sl_intr() processes the remaining interrupt types.

Bart.

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

[Sebastian Andrzej Siewior]

On 2026-07-06 07:30:48 [-0700], Bart Van Assche wrote:
> On 6/30/26 7:15 AM, Sebastian Andrzej Siewior wrote:
> >  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.
> 
> Writing into the REG_INTERRUPT_STATUS register clears the bits that are
> set in the first argument of writel().

This makes it sounds as it would acknowledge the interrupt. If that is
the case then there is no need for IRQF_ONESHOT.

> > |         /* 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 MCQ is disabled, ufshcd_sl_intr() processes all types of interrupts.
> If MCQ is enabled, the ESI interrupt handlers process I/O completions
> and ufshcd_sl_intr() processes the remaining interrupt types.

So MCQ is the trigger. Does its status change after device's init time?

If I understood it correctly, after the REG_INTERRUPT_STATUS there is no
need for IRQF_ONESHOT since this masks the interrupt until the thread is
done.

If MCQ disabled there is no need for this ACK and the it could be just a
request_irq(, ufshcd_sl_intr).

If MCQ is enabled then request_threaded_irq(, mask_interrupt_only,
   ufshcd_threaded_intr, IRQF_NO_THREAD)

Would this work?

> Bart.

Sebastian

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

[Gregory CLEMENT]

Hello Sebastian,

> On 2026-07-06 07:30:48 [-0700], Bart Van Assche wrote:
>> On 6/30/26 7:15 AM, Sebastian Andrzej Siewior wrote:
>> >  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.
>> 
>> Writing into the REG_INTERRUPT_STATUS register clears the bits that are
>> set in the first argument of writel().
>
> This makes it sounds as it would acknowledge the interrupt. If that is
> the case then there is no need for IRQF_ONESHOT.
>
>> > |         /* 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 MCQ is disabled, ufshcd_sl_intr() processes all types of interrupts.
>> If MCQ is enabled, the ESI interrupt handlers process I/O completions
>> and ufshcd_sl_intr() processes the remaining interrupt types.
>
> So MCQ is the trigger. Does its status change after device's init
> time?

The status doesn't change after the device's initialization, so we can
indeed register the interrupt according to whether MCQ is enabled or
not.

>
> If I understood it correctly, after the REG_INTERRUPT_STATUS there is no
> need for IRQF_ONESHOT since this masks the interrupt until the thread is
> done.
>
> If MCQ disabled there is no need for this ACK and the it could be just a
> request_irq(, ufshcd_sl_intr).
>
> If MCQ is enabled then request_threaded_irq(, mask_interrupt_only,
>    ufshcd_threaded_intr, IRQF_NO_THREAD)
>

I'm implementing your solution to see if it fixes the issue without
introducing a regression., but I'm puzzled by this previous line:
`mask_interrupt_only ??`.
Also, I don't understand why `IRQF_NO_THREAD` is used for the threaded
IRQ; it seems to indicate opposite behavior. Could you please explain
what you have in mind ?

Grégory

> Would this work?
>
>> Bart.
>
> Sebastian

-- 
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-07-15 17:15:08 [+0200], Gregory CLEMENT wrote:
> Hello Sebastian,
Hi,

> > If MCQ is enabled then request_threaded_irq(, mask_interrupt_only,
> >    ufshcd_threaded_intr, IRQF_NO_THREAD)
> >
> 
> I'm implementing your solution to see if it fixes the issue without
> introducing a regression., but I'm puzzled by this previous line:
> `mask_interrupt_only ??`.

mask_interrupt_only means the function would mask the interrupt in the
device (UFS) hardware so the IRQ does not raise again once interrupts
are enabled again. The ufshcd_threaded_intr() routine would then have to
unmask the interrupt at the end of its doing so the IRQ can raise again.

> Also, I don't understand why `IRQF_NO_THREAD` is used for the threaded
> IRQ; it seems to indicate opposite behavior. Could you please explain
> what you have in mind ?

The previous without IRQF_NO_THREAD provide a primary handler just
masking the IRQ line and a threaded handler. One thread visible in the
system.
With `threadirqs' or PREEMPT_RT enabled the IRQ core will thread the
primary handler (masking just the IRQ line) and the threaded handler
which does the work. So you will see two threads. Also the interrupt
line will be masked (in the IRQ-chip) as long the two handler are
running. ps will show two threads.

To limit this, IRQF_NO_THREAD will ensure that the primary handler
(which masks the IRQ line in the device's HW) will not be threaded. It
is brief, takes no locks so why not. The secondary handler is the only
thread. The IRQ will also not be masked in the IRQ-chip.

> Grégory
> 
Sebastian