From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@elte.hu>, ppc <linuxppc-dev@lists.ozlabs.org>,
Gavin Shan <shangw@linux.vnet.ibm.com>
Subject: [patch 03/26] powerpc: eeh: Kill another abuse of irq_desc
Date: Sun, 23 Feb 2014 21:40:09 -0000 [thread overview]
Message-ID: <20140223212736.562906212@linutronix.de> (raw)
In-Reply-To: 20140223212703.511977310@linutronix.de
commit 91150af3a (powerpc/eeh: Fix unbalanced enable for IRQ) is
another brilliant example of trainwreck engineering.
The patch "fixes" the issue of an unbalanced call to irq_enable()
which causes a prominent warning by checking the disabled state of the
interrupt line and call conditionally into the core code.
This is wrong in two aspects:
1) The warning is there to tell users, that they need to fix their
asymetric enable/disable patterns by finding the root cause and
solving it there.
It's definitely not meant to work around it by conditionally
calling into the core code depending on the random state of the irq
line.
Asymetric irq_disable/enable calls are a clear sign of wrong usage
of the interfaces which have to be cured at the root and not by
somehow hacking around it.
2) The abuse of core internal data structure instead of using the
proper interfaces for retrieving the information for the 'hack
around'
irq_desc is core internal and it's clear enough stated.
Replace at least the irq_desc abuse with the proper functions and add
a big fat comment why this is absurd and completely wrong.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: ppc <linuxppc-dev@lists.ozlabs.org>
---
arch/powerpc/kernel/eeh_driver.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
Index: tip/arch/powerpc/kernel/eeh_driver.c
===================================================================
--- tip.orig/arch/powerpc/kernel/eeh_driver.c
+++ tip/arch/powerpc/kernel/eeh_driver.c
@@ -143,15 +143,31 @@ static void eeh_disable_irq(struct pci_d
static void eeh_enable_irq(struct pci_dev *dev)
{
struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
- struct irq_desc *desc;
if ((edev->mode) & EEH_DEV_IRQ_DISABLED) {
edev->mode &= ~EEH_DEV_IRQ_DISABLED;
-
- desc = irq_to_desc(dev->irq);
- if (desc && desc->depth > 0)
+ /*
+ * FIXME !!!!!
+ *
+ * This is just ass backwards. This maze has
+ * unbalanced irq_enable/disable calls. So instead of
+ * finding the root cause it works around the warning
+ * in the irq_enable code by conditionally calling
+ * into it.
+ *
+ * That's just wrong.The warning in the core code is
+ * there to tell people to fix their assymetries in
+ * their own code, not by abusing the core information
+ * to avoid it.
+ *
+ * I so wish that the assymetry would be the other way
+ * round and a few more irq_disable calls render that
+ * shit unusable forever.
+ *
+ * tglx
+ */
+ if (irqd_irq_disabled(irq_get_irq_data(dev->irq))
enable_irq(dev->irq);
- }
}
/**
next prev parent reply other threads:[~2014-02-23 21:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20140223212703.511977310@linutronix.de>
2014-02-23 21:40 ` [patch 01/26] powerpc: irq: Use generic_handle_irq Thomas Gleixner
2014-03-04 16:39 ` [tip:irq/core] powerpc: Irq: " tip-bot for Thomas Gleixner
2014-02-23 21:40 ` [patch 02/26] powerpc:evh_pic: Kill irq_desc abuse Thomas Gleixner
2014-03-04 16:39 ` [tip:irq/core] powerpc:eVh_pic: " tip-bot for Thomas Gleixner
2014-02-23 21:40 ` Thomas Gleixner [this message]
2014-02-23 22:26 ` [patch 03/26] powerpc: eeh: Kill another abuse of irq_desc Benjamin Herrenschmidt
2014-02-24 7:56 ` Gavin Shan
2014-02-24 11:32 ` Thomas Gleixner
2014-03-04 16:40 ` [tip:irq/core] powerpc: Eeh: " tip-bot for Thomas Gleixner
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=20140223212736.562906212@linutronix.de \
--to=tglx@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=shangw@linux.vnet.ibm.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).