mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: linux-ide@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, marvin@mydatex.cz, tglx@linutronix.de
Subject: IRQ enable/disable BUG in IDE w/shared IRQs
Date: Thu, 13 Jan 2011 23:32:59 -0800 (PST)	[thread overview]
Message-ID: <20110113.233259.157492570.davem@davemloft.net> (raw)


As far as I know this issue has been around forever.

The bug report is at:

https://bugzilla.kernel.org/show_bug.cgi?id=16481

The important part of the backtrace is:

[    4.003171] WARNING: at kernel/irq/manage.c:290 __enable_irq+0x2f/0x55()
 ...
[    4.003176] Unbalanced enable for IRQ 19
 ...
[    4.003255]  [<c107314d>] ? __enable_irq+0x2f/0x55
[    4.003259]  [<c1073415>] ? enable_irq+0x31/0x4c
[    4.003270]  [<fb915145>] ? ide_probe_port+0x4b7/0x4de [ide_core]
[    4.003280]  [<fb915641>] ? ide_host_register+0x204/0x4e7 [ide_core]

IRQ 19 on this system is shared between the it8213 IDE controller
and one of the USB host controllers.

The condition doesn't trigger every time, only some boots, which sort
of implies a race.

The function ide_probe_port() will unconditionally do a disable_irq()/
enable_irq() around the drive probing sequence.  It does this even if
the IRQ has not been registered yet by IDE.

Normally, this is fine, since if the IRQ is not registered then
irq_desc->depth is 1 and the disable/enable sequence will behave as
essentially a NOP.  If the IRQ is registered by IDE itself, then this
sequence would really disable and then re-enable the IRQ.  That's fine
too.

But this all falls apart if the IDE interrupt line is shared and one
of those other entities does the request_irq() in the middle the
enable/disable sequence.

As far as I can tell the bad sequence is:

	ide_probe_irq			USB host controller driver

			->depth starts at "1"

	disable_irq()
	    ->depth now "2"

					request_irq()
					  ->depth reset to "0"
	enable_irq()
	    ->depth is "0", warning triggers

It would seem that there is an implicit disallowing of playing
with the enable/disable state of an IRQ until request_irq() has
been by someone first, otherwise the above sequence can trigger.

The ATA layer doesn't operate this way, it always requests the IRQ
before doing anything else wrt. interrupts on it.

I suppose I can change IDE to behave this way too, but the less
changes I make to IDE the better and there are also some subtle issues
wrt. dynamic IDE probing I need to look into to get this right.

I have a hard time believing we've gotten away with this for so long.
Maybe it really is that rare to share the IDE interrupts with other
stuff?

Any comments?


             reply	other threads:[~2011-01-14  7:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-14  7:32 David Miller [this message]
2011-01-14 10:30 ` Thomas Gleixner
2011-01-14 18:44   ` Jeff Garzik

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=20110113.233259.157492570.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marvin@mydatex.cz \
    --cc=tglx@linutronix.de \
    /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

all inboxes | Powered by JetHome®