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@xxxxxxxxxxx>
> ---
>  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