[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20150205153300.GA23903@kria>
Date: Thu, 5 Feb 2015 16:33:00 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Peter Zijlstra <peterz@...radead.org>,
David Miller <davem@...emloft.net>
Cc: Thomas Gleixner <tglx@...utronix.de>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next 00/11] net: remove disable_irq() from
->ndo_poll_controller
2015-02-05, 14:06:23 +0100, Peter Zijlstra wrote:
> On Thu, Feb 05, 2015 at 01:20:59AM +0100, Sabrina Dubroca wrote:
> > Thomas, ping?
> >
> > thread is over there if you need it:
> > https://marc.info/?l=linux-netdev&m=141833435000554&w=2
>
> It appears to me you have addressed most his issues and since it appears
> to me the existing synchronze_hardirq() seems to be mostly what we need
> I'll propose (and merge) the below and will take flames from tglx when
> he returns :-)
Great, thanks a lot Peter!
I will prepare the drivers patches based on this.
David, do you want one patch for each driver, or do you want me to group the changes?
There are 60+ drivers to patch, and almost all the patches are just:
- disable_irq(irq);
- handler(a, b);
+ if (disable_hardirq(irq))
+ handler(a,b);
> ---
> Subject: genirq: Provide disable_hardirq()
>
> For things like netpoll there is a need to disable an interrupt from
> atomic context. Currently netpoll uses disable_irq() which will
> sleep-wait on threaded handlers and thus forced_irqthreads breaks
> things.
>
> Provide disable_hardirq(), which uses synchronize_hardirq() to only wait
> for active hardirq handlers; also change synchronize_hardirq() to
> return the status of threaded handlers.
>
> This will allow one to try-disable an interrupt from atomic context, or
> in case of request_threaded_irq() to only wait for the hardirq part.
>
> Suggested-by: Sabrina Dubroca <sd@...asysnail.net>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
> include/linux/hardirq.h | 2 +-
> include/linux/interrupt.h | 1 +
> kernel/irq/manage.c | 36 ++++++++++++++++++++++++++++++++++--
> 3 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
> index cba442ec3c66..f4af03404b97 100644
> --- a/include/linux/hardirq.h
> +++ b/include/linux/hardirq.h
> @@ -9,7 +9,7 @@
>
>
> extern void synchronize_irq(unsigned int irq);
> -extern void synchronize_hardirq(unsigned int irq);
> +extern bool synchronize_hardirq(unsigned int irq);
>
> #if defined(CONFIG_TINY_RCU)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index d9b05b5bf8c7..3bb01b9a379c 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -184,6 +184,7 @@ extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
> #endif
>
> extern void disable_irq_nosync(unsigned int irq);
> +extern bool disable_hardirq(unsigned int irq);
> extern void disable_irq(unsigned int irq);
> extern void disable_percpu_irq(unsigned int irq);
> extern void enable_irq(unsigned int irq);
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index f038e586a4b9..1309cccd714f 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -68,14 +68,20 @@ static void __synchronize_hardirq(struct irq_desc *desc)
> * Do not use this for shutdown scenarios where you must be sure
> * that all parts (hardirq and threaded handler) have completed.
> *
> + * Returns: false if a theaded handler is active.
> + *
> * This function may be called - with care - from IRQ context.
> */
> -void synchronize_hardirq(unsigned int irq)
> +bool synchronize_hardirq(unsigned int irq)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - if (desc)
> + if (desc) {
> __synchronize_hardirq(desc);
> + return !atomic_read(&desc->threads_active);
> + }
> +
> + return true;
> }
> EXPORT_SYMBOL(synchronize_hardirq);
>
> @@ -439,6 +445,32 @@ void disable_irq(unsigned int irq)
> }
> EXPORT_SYMBOL(disable_irq);
>
> +/**
> + * disable_hardirq - disables an irq and waits for hardirq completion
> + * @irq: Interrupt to disable
> + *
> + * Disable the selected interrupt line. Enables and Disables are
> + * nested.
> + * This function waits for any pending hard IRQ handlers for this
> + * interrupt to complete before returning. If you use this function while
> + * holding a resource the hard IRQ handler may need you will deadlock.
> + *
> + * When used to optimistically disable an interrupt from atomic context
> + * the return value must be checked.
> + *
> + * Returns: false if a theaded handler is active.
> + *
> + * This function may be called - with care - from IRQ context.
> + */
> +bool disable_hardirq(unsigned int irq)
> +{
> + if (!__disable_irq_nosync(irq))
> + return synchronize_hardirq(irq);
> +
> + return false;
> +}
> +EXPORT_SYMBOL(disable_hardirq);
> +
> void __enable_irq(struct irq_desc *desc, unsigned int irq)
> {
> switch (desc->depth) {
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sabrina
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists