From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
rtc-linux@googlegroups.com, intel-gfx@lists.freedesktop.org,
Gabriele Mazzotta <gabriele.mzt@gmail.com>,
Alexandre Belloni <alexandre.belloni@free-electrons.com>,
linux-kernel@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH] rtc: cmos: Don't enable interrupts in the middle of the interrupt handler
Date: Wed, 19 Oct 2016 21:33:10 +0300 [thread overview]
Message-ID: <20161019183310.GL4329@intel.com> (raw)
In-Reply-To: <20161019181639.GB10824@nuc-i3427.alporthouse.com>
On Wed, Oct 19, 2016 at 07:16:39PM +0100, Chris Wilson wrote:
> On Wed, Oct 19, 2016 at 09:02:04PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Using spin_lock_irq()/spin_unlock_irq() from within the interrupt
> > handler is a no-no. Let's save/restore the flags to avoid turning on
> > interrupts prematurely.
> >
> > We hit this in a bunch of our CI systems, but for whatever reason I
> > wasn't able to reproduce on my own machine, so this fix is just
> > based on the backtrace.
> >
> > [ 202.634918] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2729 trace_hardirqs_on_caller+0x113/0x1b0
> > [ 202.634919] DEBUG_LOCKS_WARN_ON(current->hardirq_context)
> > [ 202.634929] Modules linked in: snd_hda_intel i915 x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel lpc_ich snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_codec snd_hwdep i2c_designware_platform i2c_designware_core snd_hda_core mei_me mei snd_pcm r8169 mii sdhci_acpi sdhci mmc_core i2c_hid [last unloaded: i915]
> > [ 202.634930] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G U 4.9.0-rc1-CI-CI_DRM_1734+ #1
> > [ 202.634931] Hardware name: GIGABYTE M4HM87P-00/M4HM87P-00, BIOS F6 12/10/2014
> > [ 202.634933] ffff88011ea03d68 ffffffff8142dce5 ffff88011ea03db8 0000000000000000
> > [ 202.634934] ffff88011ea03da8 ffffffff8107e496 00000aa900000002 ffffffff81e249a0
> > [ 202.634935] ffffffff81815637 ffffffff82e7c280 0000000000000000 0000000000000004
> > [ 202.634936] Call Trace:
> > [ 202.634939] <IRQ>
> > [ 202.634939] [<ffffffff8142dce5>] dump_stack+0x67/0x92
> > [ 202.634941] [<ffffffff8107e496>] __warn+0xc6/0xe0
> > [ 202.634944] [<ffffffff81815637>] ? _raw_spin_unlock_irq+0x27/0x50
> > [ 202.634945] [<ffffffff8107e4fa>] warn_slowpath_fmt+0x4a/0x50
> > [ 202.634946] [<ffffffff810d6d83>] trace_hardirqs_on_caller+0x113/0x1b0
> > [ 202.634948] [<ffffffff810d6e2d>] trace_hardirqs_on+0xd/0x10
> > [ 202.634949] [<ffffffff81815637>] _raw_spin_unlock_irq+0x27/0x50
> > [ 202.634951] [<ffffffff81672042>] rtc_handler+0x32/0xa0
> > [ 202.634954] [<ffffffff814c08a3>] acpi_ev_fixed_event_detect+0xd4/0xfb
> > [ 202.634956] [<ffffffff814c2ccb>] acpi_ev_sci_xrupt_handler+0xf/0x2d
> > [ 202.634957] [<ffffffff814ab3ee>] acpi_irq+0x11/0x2c
> > [ 202.634960] [<ffffffff810e5288>] __handle_irq_event_percpu+0x58/0x370
> > [ 202.634961] [<ffffffff810e55be>] handle_irq_event_percpu+0x1e/0x50
> > [ 202.634962] [<ffffffff810e5624>] handle_irq_event+0x34/0x60
> > [ 202.634963] [<ffffffff810e8906>] handle_fasteoi_irq+0xa6/0x170
> > [ 202.634966] [<ffffffff8101eef5>] handle_irq+0x15/0x20
> > [ 202.634967] [<ffffffff8101e548>] do_IRQ+0x68/0x130
> > [ 202.634968] [<ffffffff81816789>] common_interrupt+0x89/0x89
> > [ 202.634970] <EOI>
> > [ 202.634970] [<ffffffff81814c73>] ? mwait_idle+0x93/0x210
> > [ 202.634971] [<ffffffff81814c6a>] ? mwait_idle+0x8a/0x210
> > [ 202.634972] [<ffffffff81026b0a>] arch_cpu_idle+0xa/0x10
> > [ 202.634973] [<ffffffff8181509e>] default_idle_call+0x1e/0x30
> > [ 202.634974] [<ffffffff810cbf6c>] cpu_startup_entry+0x17c/0x1f0
> > [ 202.634976] [<ffffffff8180ca87>] rest_init+0x127/0x130
> > [ 202.634978] [<ffffffff81f77f08>] start_kernel+0x3f6/0x403
> > [ 202.634980] [<ffffffff81f7728f>] x86_64_start_reservations+0x2a/0x2c
> > [ 202.634981] [<ffffffff81f77404>] x86_64_start_kernel+0x173/0x186
> > [ 202.634982] ---[ end trace 293c99618fa08d34 ]---
> >
> > Cc: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > Fixes: 983bf1256edb ("rtc: cmos: Clear ACPI-driven alarms upon resume")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/rtc/rtc-cmos.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
> > index dd3d59806ffa..19cd49ad92dc 100644
> > --- a/drivers/rtc/rtc-cmos.c
> > +++ b/drivers/rtc/rtc-cmos.c
> > @@ -996,8 +996,9 @@ static u32 rtc_handler(void *context)
> > struct cmos_rtc *cmos = dev_get_drvdata(dev);
> > unsigned char rtc_control = 0;
> > unsigned char rtc_intr;
> > + unsigned long flags;
> >
> > - spin_lock_irq(&rtc_lock);
> > + spin_lock_irqsave(&rtc_lock, flags);
>
> My reading of the acpi fixed event handlers suggests that this can only
> be called from interrupt context.
Ah, could be. I didn't bother digging quite that deep.
>
> Anyway using irqsave is correct (just may be optimised away to a plain
> spin_lock() if the condition that this is only called in irq context is
> satisfied).
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2016-10-19 18:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-19 18:02 [PATCH] rtc: cmos: Don't enable interrupts in the middle of the interrupt handler ville.syrjala
2016-10-19 18:16 ` [Intel-gfx] " Chris Wilson
2016-10-19 18:33 ` Ville Syrjälä [this message]
2016-10-20 15:40 ` Alexandre Belloni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161019183310.GL4329@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=alexandre.belloni@free-electrons.com \
--cc=chris@chris-wilson.co.uk \
--cc=gabriele.mzt@gmail.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rtc-linux@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).