Re: [PATCH] irq: Resolve that mask_irq/unmask_irq may not be called in pairs

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Dec 07 2023 at 09:40, xiongxin@kylinos.cn wrote:
> When an interrupt controller uses a function such as handle_level_irq()
> as an interrupt handler and the controller implements the irq_disable()
> callback, the following scenario will appear in the i2c-hid driver in
> the sleep scenario:
>
> in the sleep flow, while the user is still triggering the i2c-hid
> interrupt, we get the following function call:
>
>   handle_level_irq()
>     -> mask_ack_irq()
>       -> mask_irq()
> 				i2c_hid_core_suspend()
> 				  -> disable_irq()
> 				    -> __irq_disable()
> 				      -> irq_state_set_disabled()
> 				      -> irq_state_set_masked()
>
>   irq_thread_fn()
>     -> irq_finalize_oneshot()
>       -> if (!desc->threads_oneshot && !irqd_irq_disabled() &&
> 	     irqd_irq_masked())
>       	 	unmask_threaded_irq()
> 		  -> unmask_irq()
>
> That is, when __irq_disable() is called between mask_irq() and
> irq_finalize_oneshot(), the code in irq_finalize_oneshot() will cause
> the !irqd_irq_disabled() fails to enter the unmask_irq() branch, which
> causes mask_irq/unmask_irq to be called unpaired and the i2c-hid
> interrupt to be masked.
>
> Since mask_irq/unmask_irq and irq_disabled() belong to two different
> hardware registers or policies, the !irqd_irq_disabled() assertion may
> not be used to determine whether unmask_irq() needs to be called.

No. That's fundamentally wrong.

Disabled interrupts are disabled and can only be reenabled by the
corresponding enable call. The existing code is entirely correct.

What you are trying to do is unmasking a disabled interrupt, which
results in inconsistent state.

Which interrupt chip is involved here?

Thanks,

        tglx