On Monday 02 March 2009, Peter Zijlstra wrote:
> On Mon, 2009-03-02 at 14:10 -0800, David Brownell wrote:
>
> > What's unfortunate is that you prefer not to fix that
> > IRQF_DISABLED bug in lockdep, which you co-"maintain".
> > When running with lockdep, that bug (a) introduces bugs
> > in some drivers and (b) hides bugs in others. You've
> > rejected even a minimal warning fix, to help minimize
> > the amount of time developers waste on (a) and (b).
>
> I've come to the conclusion that the only technically sound solution is
> to do as I proposed today, utterly eliminate !IRQF_DISABLED handlers.
As you announced today. If you truly believe that, then
you should at least submit a warning patch for 2.6.29-rc
("driver X isn't setting IRQF_DISABLED, reimplement!")
with a Documentation/feature-removal-schedule.txt plan
for removing that mechanism. And maybe updating the
handling of IRQF_SHARED at the same time... most shared
IRQs don't want IRQF_DISABLED. And ... surely more,
even just to start phasing out such a longstanding
mechanism.
(Or more likely, start a real LKML discussion on that
specific topic. I suspect it won't complete in time
to merge such a patch with any sort of consensus.)
> Apparently you had enough time to come up with the creative genirq abuse
> of twl4030, I think that with a similar effort you could have
> implemented generic threaded irq stuff like proposed by Thomas.
Actually, I made time to clean that code up a lot;
I didn't come up with that original stuff at all.
Why would you think otherwise, after reading the
copyrights at the top of twl4030-irq.c ??
And when I was doing that cleanup, the Official Plan
was that Thomas' code would be ready for merge into
at least the -next tree soon. I don't know about
you, but stepping all over his work didn't seem like
it would be at all constructive. Having a significant
driver be ready to try using it ... seemed like a
more productive approach.
> > Attacking folk for having to cope with such bugs escalates
> > things beyond "unfortunate". If lockdep is "maintained",
> > your response should be fixing that lockdep bug. Once
> > that's done, all workarounds for that bug can be removed.
>
> I state there is no lockdep bug in this respect. The bug is trying to
> enable interrupts from hardirq context and running code that assumes
> hardirq context from task context.
Well, I state that you're wrong about those points...
Regardless of what either of us states, the truth of
the matter is that there are drivers that don't work
with CONFIG_LOCKDEP and the *only* reason is the code
removed by the (untested) patchlet below: semantics
of irqs change when lockdep is enabled. (That is, your
characterization above is incorrect.) I call that a
lockdep bug; you don't want to accept that label.
Are those incorrect lockdep assumptions really so hard
to fix? Where do they surface? I scanned lockdep.c
briefly, and it talks about "hardirq" in several places.
Is it conflating that with "irqs disabled"? The two
concepts should be distinct.
- Dave
---
kernel/irq/manage.c | 6 ------
1 file changed, 6 deletions(-)
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -816,12 +816,6 @@ int request_irq_quickcheck(unsigned int
"guaranteed on shared IRQs\n",
irq, devname);
-#ifdef CONFIG_LOCKDEP
- /*
- * Lockdep wants atomic interrupt handlers:
- */
- irqflags |= IRQF_DISABLED;
-#endif
/*
* Sanity-check: shared interrupts must pass in a real dev-ID,
* otherwise we'll have trouble later trying to figure out
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
