linux-kernel - Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with IRQF_NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150306110618.GC8700@leverpostej>
Date:	Fri, 6 Mar 2015 11:06:18 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	"Rafael J. Wysocki" <rafael@...nel.org>,
	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	Peter Zijlstra <peterz@...radead.org>,
	Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Wim Van Sebroeck <wim@...ana.be>,
	"linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	"rtc-linux@...glegroups.com" <rtc-linux@...glegroups.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Mike Turquette <mturquette@...aro.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 5/6] watchdog: at91sam9: request the irq with
 IRQF_NO_SUSPEND

[...]

> > The request_irq path never results in a call to chip->irq_set_wake(),
> > even with the IRQF_NO_SUSPEND flag. So requesting an irq with
> > IRQF_NO_SUSPEND does not guarantee wakeup; it only guarantees that the
> > CPU can take the interrupt _around_ the suspended state, not necessarily
> > while _in_ the suspended state.
> 
> Right.  "Suspended state" meaning full suspend here I suppose?

Yes; any state deeper than suspend-to-idle.

[...]

> > We seem to be conflating some related properties:
> > 
> > [a] The IRQ will be left unmasked.
> > [b] The IRQ will be handled immediately when taken.
> > [c] The IRQ will wake the system from suspend.
> > 
> > Requesting an IRQ with IRQF_NO_SUSPEND guarantees [a,b], but does not
> > guarantee [c].
> 
> That's correct.  IRQF_NO_SUSPEND does not guarantee that interrupts from
> that IRQ will have any effect after arch_suspend_disable_irqs() in
> suspend_enter().

[...]

> > It sounds like for this kind of watchdog device we want [a,b,c], even if
> > the IRQ is not shared with an IRQF_NO_SUSPEND user.
> 
> We can't guarantee that, though.  arch_suspend_disable_irqs() disables
> interrupts on the last working CPU and it won't get any.  It may be
> brought out of a low-power state by a pending interrupt, but it won't act
> upon that interrupt immediately anyway, only after the arch_suspend_enable_irqs()
> in suspend_enter().

Ok, so [b] needs the caveat that it's only handled "immediately" outside
of the arch_suspend_disable_irqs() ... arch_suspend_enable_irqs()
section.

> But then it might as well be deferred until after
> resume_device_irqs().

That was my original line of thinking, in which case the watchdog driver
should use IRQF_COND_SUSPEND rather than IRQF_NO_SUSPEND, with
enable_irq_wake() if we care about the watchdog during suspend. I'm
happy with this.

Considering that the use-case of a watchdog is to alert us to something
going hideously wrong in the kernel, we want to handle the IRQ after
executing the smallest amount of kernel code possible. For that, they
need to have their handlers to be called "immediately" outside of the
arch_suspend_disable_irqs() ... arch_suspend_enable_irqs() window, and
need to be enabled during suspend to attempt to catch bad wakeup device
configuration.

I think it's possible (assuming the caveats on [b] above) to provide
[a,b,c] for this case.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/