lkml.org 
[lkml]   [2022]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] irq/core: synchronize irq_thread startup


On Mon, 2 May 2022, Thomas Gleixner wrote:

> But free_irq() stopped issuing synchronize_irq() with commit
> 519cc8652b3a ("genirq: Synchronize only with single thread on
> free_irq()"). And that turns out to be the root cause of the problem.
> I should have caught that back then, but in hindsight ....
>
> While the proposed patch works, I think the real solution is to ensure
> that both the hardware interrupt _and_ the interrupt threads which are
> associated to the removed action are in quiescent state. This should
> catch the case you observed.

I can confirm that your patch works.
And it also explains why I never had this issue on a 4.4 kernel with
realtime patch ...

> ---
> Subject: genirq: Quiesce interrupt threads in free_irq()
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Mon, 02 May 2022 15:40:25 +0200
>
> Fill void...
>
> Fixes: 519cc8652b3a ("genirq: Synchronize only with single thread on free_irq()")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> kernel/irq/manage.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1914,6 +1914,22 @@ static struct irqaction *__free_irq(stru
> */
> __synchronize_hardirq(desc, true);
>
> + /*
> + * Wait for associated interrupt threads to complete. This cannot
> + * use synchronize_irq() due to interrupt sharing in the PCIe
> + * layer. See 519cc8652b3a ("genirq: Synchronize only with single
> + * thread on free_irq()") for further explanation.
> + */
> + if (action->thread) {
> + unsigned int thread_mask = action->thread_mask;
> +
> + if (action->secondary)
> + thread_mask |= action->secondary->thread_mask;
> +
> + wait_event(desc->wait_for_threads,
> + !(atomic_read(&desc->threads_active) & thread_mask));
> + }
> +
> #ifdef CONFIG_DEBUG_SHIRQ
> /*
> * It's a shared IRQ -- the driver ought to be prepared for an IRQ
> @@ -1931,10 +1947,11 @@ static struct irqaction *__free_irq(stru
> #endif
>
> /*
> - * The action has already been removed above, but the thread writes
> - * its oneshot mask bit when it completes. Though request_mutex is
> - * held across this which prevents __setup_irq() from handing out
> - * the same bit to a newly requested action.
> + * The action has already been removed above and both the hardware
> + * interrupt and the associated threads have been synchronized,
> + * which means they are in quiescent state. request_mutex is still
> + * held which prevents __setup_irq() from handing out action's
> + * thread_mask to a newly requested action.
> */
> if (action->thread) {
> kthread_stop(action->thread);
>
>
>

Tested-by: Thomas Pfaff <tpfaff@pcs.com>

Thank you,
Thomas


\
 
 \ /
  Last update: 2022-05-02 17:35    [W:0.080 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site