When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
sure the driver is ready to receive an IRQ immediately. The problem was that
this fake IRQ was being executed even if interrupt line failed to be allocated
by __setup_irq.
Signed-off-by: Luis Henriques <[email protected]>
---
kernel/irq/manage.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index a3eb7ba..39a0f4d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -914,7 +914,7 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
kfree(action);
#ifdef CONFIG_DEBUG_SHIRQ
- if (irqflags & IRQF_SHARED) {
+ if (!retval & (irqflags & IRQF_SHARED)) {
/*
* It's a shared IRQ -- the driver ought to be prepared for it
* to happen immediately, so let's make sure....
--
1.6.2.1
Commit-ID: ba71604fad348656071a2a76eef9a67dab85a773
Gitweb: http://git.kernel.org/tip/ba71604fad348656071a2a76eef9a67dab85a773
Author: Luis Henriques <[email protected]>
AuthorDate: Wed, 1 Apr 2009 18:06:35 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 2 Apr 2009 16:02:39 +0200
genirq: do not execute DEBUG_SHIRQ when irq setup failed
When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
sure the driver is ready to receive an IRQ immediately. The problem was that
this fake IRQ was being executed even if interrupt line failed to be allocated
by __setup_irq.
Signed-off-by: Luis Henriques <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/irq/manage.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1516ab7..beeb7d1 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -768,7 +768,7 @@ int request_irq(unsigned int irq, irq_handler_t handler,
kfree(action);
#ifdef CONFIG_DEBUG_SHIRQ
- if (irqflags & IRQF_SHARED) {
+ if (!retval & (irqflags & IRQF_SHARED)) {
/*
* It's a shared IRQ -- the driver ought to be prepared for it
* to happen immediately, so let's make sure....
On Thu, 2009-04-02 at 14:03 +0000, Luis Henriques wrote:
> Commit-ID: ba71604fad348656071a2a76eef9a67dab85a773
> Gitweb: http://git.kernel.org/tip/ba71604fad348656071a2a76eef9a67dab85a773
> Author: Luis Henriques <[email protected]>
> AuthorDate: Wed, 1 Apr 2009 18:06:35 +0100
> Committer: Thomas Gleixner <[email protected]>
> CommitDate: Thu, 2 Apr 2009 16:02:39 +0200
>
> genirq: do not execute DEBUG_SHIRQ when irq setup failed
>
> When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
> sure the driver is ready to receive an IRQ immediately. The problem was that
> this fake IRQ was being executed even if interrupt line failed to be allocated
> by __setup_irq.
>
> Signed-off-by: Luis Henriques <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
>
>
> ---
> kernel/irq/manage.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 1516ab7..beeb7d1 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -768,7 +768,7 @@ int request_irq(unsigned int irq, irq_handler_t handler,
> kfree(action);
>
> #ifdef CONFIG_DEBUG_SHIRQ
> - if (irqflags & IRQF_SHARED) {
> + if (!retval & (irqflags & IRQF_SHARED)) {
> /*
> * It's a shared IRQ -- the driver ought to be prepared for it
> * to happen immediately, so let's make sure....
What is this ?
There is no retval:
http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
*/
696 static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
697 {
698 struct irq_desc *desc = irq_to_desc(irq);
699 struct irqaction *action, **action_ptr;
700 struct task_struct *irqthread;
701 unsigned long flags;
702
703 WARN(in_interrupt(), "Trying to free IRQ %
d from IRQ context!\n", irq);
704
705 if (!desc)
706 return NULL;
707
708 spin_lock_irqsave(&desc->lock, flags);
709
710 /*
711
* There can be multiple actions per IRQ descriptor, find the right
712 * one based on the dev_id:
713 */
714 action_ptr = &desc->action;
715 for (;;) {
716 action = *action_ptr;
717
718 if (!action) {
719 WARN(1, "Trying to free already-free IRQ %d
\n", irq);
720 spin_unlock_irqrestore(&desc->lock, flags);
721
722 return NULL;
723 }
724
725 if (action->dev_id == dev_id)
726 break;
727 action_ptr = &action->next;
728 }
729
730 /* Found it - now remove it from the list of entries: */
731 *action_ptr = action->next;
732
733 /* Currently used only by UML, might disappear one day: */
734 #ifdef CONFIG_IRQ_RELEASE_METHOD
735 if (desc->chip->release)
736 desc->chip->release(irq, dev_id);
737 #endif
738
739 /* If this was the last handler, shut down the IRQ line: */
740 if (!desc->action) {
741 desc->status |= IRQ_DISABLED;
742 if (desc->chip->shutdown)
743 desc->chip->shutdown(irq);
744 else
745 desc->chip->disable(irq);
746 }
747
748 irqthread = action->thread;
749 action->thread = NULL;
750
751 spin_unlock_irqrestore(&desc->lock, flags);
752
753 unregister_handler_proc(irq, action);
754
755 /* Make sure it's not being used on another CPU: */
756 synchronize_irq(irq);
757
758 if (irqthread) {
759 if (!test_bit(IRQTF_DIED, &action->thread_flags))
760 kthread_stop(irqthread);
761 put_task_struct(irqthread);
762 }
763
764 #ifdef CONFIG_DEBUG_SHIRQ
765 /*
766
* It's a shared IRQ -- the driver ought to be prepared for an IRQ
767
* event to happen even now it's being freed, so let's make sure that
768 * is so by doing an extra call to the handler ....
769 *
770
* ( We do this after actually deregistering it, to make sure that a
771 * 'real' IRQ doesn't run in * parallel with our fake. )
772 */
773 if (action->flags & IRQF_SHARED) {
774 local_irq_save(flags);
775 action->handler(irq, dev_id);
776 local_irq_restore(flags);
777 }
778 #endif
779 return action;
780 }
On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> On Thu, 2009-04-02 at 14:03 +0000, Luis Henriques wrote:
> > Commit-ID: ba71604fad348656071a2a76eef9a67dab85a773
> > Gitweb: http://git.kernel.org/tip/ba71604fad348656071a2a76eef9a67dab85a773
> > Author: Luis Henriques <[email protected]>
> > AuthorDate: Wed, 1 Apr 2009 18:06:35 +0100
> > Committer: Thomas Gleixner <[email protected]>
> > CommitDate: Thu, 2 Apr 2009 16:02:39 +0200
> >
> > genirq: do not execute DEBUG_SHIRQ when irq setup failed
> >
> > When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
> > sure the driver is ready to receive an IRQ immediately. The problem was that
> > this fake IRQ was being executed even if interrupt line failed to be allocated
> > by __setup_irq.
> >
> > Signed-off-by: Luis Henriques <[email protected]>
> > LKML-Reference: <[email protected]>
> > Signed-off-by: Thomas Gleixner <[email protected]>
> >
> >
> > ---
> > kernel/irq/manage.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 1516ab7..beeb7d1 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -768,7 +768,7 @@ int request_irq(unsigned int irq, irq_handler_t handler,
> > kfree(action);
> >
> > #ifdef CONFIG_DEBUG_SHIRQ
> > - if (irqflags & IRQF_SHARED) {
> > + if (!retval & (irqflags & IRQF_SHARED)) {
> > /*
> > * It's a shared IRQ -- the driver ought to be prepared for it
> > * to happen immediately, so let's make sure....
>
> What is this ?
You looking at the wrong place.
> There is no retval:
>
> http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
Care to read patches you want to comment on carefully _BEFORE_ you
start yelling at people and sending useless copies of the wrong
function around the world.
The patch is perfectly fine and already applied.
Thanks,
tglx
On Thu, 2009-04-02 at 17:21 +0200, Thomas Gleixner wrote:
> On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> > On Thu, 2009-04-02 at 14:03 +0000, Luis Henriques wrote:
> > > Commit-ID: ba71604fad348656071a2a76eef9a67dab85a773
> > > Gitweb: http://git.kernel.org/tip/ba71604fad348656071a2a76eef9a67dab85a773
> > > Author: Luis Henriques <[email protected]>
> > > AuthorDate: Wed, 1 Apr 2009 18:06:35 +0100
> > > Committer: Thomas Gleixner <[email protected]>
> > > CommitDate: Thu, 2 Apr 2009 16:02:39 +0200
> > >
> > > genirq: do not execute DEBUG_SHIRQ when irq setup failed
> > >
> > > When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
> > > sure the driver is ready to receive an IRQ immediately. The problem was that
> > > this fake IRQ was being executed even if interrupt line failed to be allocated
> > > by __setup_irq.
> > >
> > > Signed-off-by: Luis Henriques <[email protected]>
> > > LKML-Reference: <[email protected]>
> > > Signed-off-by: Thomas Gleixner <[email protected]>
> > >
> > >
> > > ---
> > > kernel/irq/manage.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > > index 1516ab7..beeb7d1 100644
> > > --- a/kernel/irq/manage.c
> > > +++ b/kernel/irq/manage.c
> > > @@ -768,7 +768,7 @@ int request_irq(unsigned int irq, irq_handler_t handler,
> > > kfree(action);
> > >
> > > #ifdef CONFIG_DEBUG_SHIRQ
> > > - if (irqflags & IRQF_SHARED) {
> > > + if (!retval & (irqflags & IRQF_SHARED)) {
> > > /*
> > > * It's a shared IRQ -- the driver ought to be prepared for it
> > > * to happen immediately, so let's make sure....
> >
> > What is this ?
>
> You looking at the wrong place.
>
> > There is no retval:
> >
> > http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
>
> Care to read patches you want to comment on carefully _BEFORE_ you
> start yelling at people and sending useless copies of the wrong
> function around the world.
>
> The patch is perfectly fine and already applied.
>
Yes, I know somehow you applied it. When you will merge this branch with
-tip/master then you will understand what I am saying.
Even function name is changed from:
713 int request_irq(unsigned int irq, irq_handler_t handler,
714
unsigned long irqflags, const char *devname, void *dev_id)
715 {
716 struct irqaction *action;
717 struct irq_desc *desc;
718 int retval;
to:
857 int request_threaded_irq(unsigned int irq, irq_handler_t handler,
858
irq_handler_t thread_fn, unsigned long irqflags,
859 const char *devname, void *dev_id)
860 {
861 struct irqaction *action;
862 struct irq_desc *desc;
863 int retval;
--
JSR
On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> > > > #ifdef CONFIG_DEBUG_SHIRQ
> > > > - if (irqflags & IRQF_SHARED) {
> > > > + if (!retval & (irqflags & IRQF_SHARED)) {
> > > > /*
> > > > * It's a shared IRQ -- the driver ought to be prepared for it
> > > > * to happen immediately, so let's make sure....
> > >
> > > What is this ?
> >
> > You looking at the wrong place.
> >
> > > There is no retval:
> > >
> > > http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
> >
> > Care to read patches you want to comment on carefully _BEFORE_ you
> > start yelling at people and sending useless copies of the wrong
> > function around the world.
> >
> > The patch is perfectly fine and already applied.
> >
>
> Yes, I know somehow you applied it. When you will merge this branch with
> -tip/master then you will understand what I am saying.
Jaswinder. I really start to get annoyed.
That patch applies fine on master as well.
> Even function name is changed from:
>
> 713 int request_irq(unsigned int irq, irq_handler_t handler,
>
> 857 int request_threaded_irq(unsigned int irq, irq_handler_t handler,
And why is this fcking relevant ?
Take the patch and try to apply it. It applies perfectly fine on
tip/master. And it is still _CORRECT_ there.
If you don't know how to read a patch and don't know the code it
applies to then taking it and applying it is the minimum you could do
before wasting everybodys time.
Stop this nonsense before I get really grumpy,
tglx
On Thu, 2009-04-02 at 18:08 +0200, Thomas Gleixner wrote:
> On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> > > > > #ifdef CONFIG_DEBUG_SHIRQ
> > > > > - if (irqflags & IRQF_SHARED) {
> > > > > + if (!retval & (irqflags & IRQF_SHARED)) {
> > > > > /*
> > > > > * It's a shared IRQ -- the driver ought to be prepared for it
> > > > > * to happen immediately, so let's make sure....
> > > >
> > > > What is this ?
> > >
> > > You looking at the wrong place.
> > >
> > > > There is no retval:
> > > >
> > > > http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
> > >
> > > Care to read patches you want to comment on carefully _BEFORE_ you
> > > start yelling at people and sending useless copies of the wrong
> > > function around the world.
> > >
> > > The patch is perfectly fine and already applied.
> > >
> >
> > Yes, I know somehow you applied it. When you will merge this branch with
> > -tip/master then you will understand what I am saying.
>
> Jaswinder. I really start to get annoyed.
>
> That patch applies fine on master as well.
>
> > Even function name is changed from:
> >
> > 713 int request_irq(unsigned int irq, irq_handler_t handler,
> >
> > 857 int request_threaded_irq(unsigned int irq, irq_handler_t handler,
>
> And why is this fcking relevant ?
>
I am dead sure, you really need some good manners and need to learn many
things.
> Take the patch and try to apply it. It applies perfectly fine on
> tip/master. And it is still _CORRECT_ there.
>
> If you don't know how to read a patch and don't know the code it
> applies to then taking it and applying it is the minimum you could do
> before wasting everybodys time.
>
> Stop this nonsense before I get really grumpy,
>
That's the only thing you can do.
--
JSR
On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
>
> I am dead sure, you really need some good manners and need to learn many
> things.
Ok. Now it's definitly enough.
You started yelling at the patch submitter in an unfriendly and
totally unjustified way. And your analysis of the patch was completely
wrong as well.
I pointed that out to you and instead of thinking for a minute you
insist on your completely wrong analysis and tell me another time
about the code which I know probably better than most of the kernel
hackers.
The question arises who needs manners and who needs to learn things.
tglx
On Thu, 2009-04-02 at 18:38 +0200, Thomas Gleixner wrote:
> On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> >
> > I am dead sure, you really need some good manners and need to learn many
> > things.
>
> Ok. Now it's definitly enough.
>
> You started yelling at the patch submitter
I do not yell at patch submitter, I yell at you. Like you always do on
anothers without any reason.
> in an unfriendly and
unfriendly, do you know what is the meaning of friendly ?
> The question arises who needs manners and who needs to learn things.
>
-----> tglx
Uau! I have never seen such a trivial patch (although first version of it was
stupidly buggy!) causing this kind of discussions! :-)
Anyway, I believe the tech-related part of the initial question has been solved
(sorry for taking so long to read/reply). With respect to the non-tech-part...
I'm out :-)
--
Luis Henriques
Jaswinder,
Let me transcribe what happened:
* Jaswinder Singh Rajput <[email protected]> wrote:
> On Thu, 2009-04-02 at 18:08 +0200, Thomas Gleixner wrote:
> > On Thu, 2 Apr 2009, Jaswinder Singh Rajput wrote:
> > > > > > #ifdef CONFIG_DEBUG_SHIRQ
> > > > > > - if (irqflags & IRQF_SHARED) {
> > > > > > + if (!retval & (irqflags & IRQF_SHARED)) {
> > > > > > /*
> > > > > > * It's a shared IRQ -- the driver ought to be prepared for it
> > > > > > * to happen immediately, so let's make sure....
> > > > >
> > > > > What is this ?
[ Jaswinder misunderstands a patch and asks a rather stupid question
in a demanding tone and does not go into any level of detail why
he thinks the patch is wrong. Just a single look into the source
code file in question would have shown him his mistake. ]
> > > >
> > > > You looking at the wrong place.
[ Thomas, the genirq maintainer, points out Jaswinder's error
calmly. ]
> > > >
> > > > > There is no retval:
> > > > >
> > > > > http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=blob;f=kernel/irq/manage.c;h=a3eb7baf1e46f2c735edb4cc44e0386cfbc4989e;hb=HEAD
> > > >
> > > > Care to read patches you want to comment on carefully _BEFORE_ you
> > > > start yelling at people and sending useless copies of the wrong
> > > > function around the world.
> > > >
> > > > The patch is perfectly fine and already applied.
[ Thomas also asks Jaswinder to think things through before wasting
other people's time. ]
> > > >
> > >
> > > Yes, I know somehow you applied it. When you will merge this branch with
> > > -tip/master then you will understand what I am saying.
[ Jaswinder is being difficult: he writes that Thomas "somehow"
applied the patch - ignoring the fact that Thomas is the
maintainer of this code.
Jaswinder is also suggesting in a condescending tone that Thomas
does not understand the issue - while the code is perfectly fine
and it is Jaswinder who is trivially wrong.
Wasting more of Thomas's time. ]
> > Jaswinder. I really start to get annoyed.
> >
[ Thomas, understandably, being into the 4th mail of a thread that
should not have happened at all, is getting annoyed. ]
> > That patch applies fine on master as well.
[ Thomas points out another mistake in Jaswinder's argument. ]
> > > Even function name is changed from:
> > >
> > > 713 int request_irq(unsigned int irq, irq_handler_t handler,
> > >
> > > 857 int request_threaded_irq(unsigned int irq, irq_handler_t handler,
[ Jaswinder shows evidence of even more stupidity: he points out to
Thomas that a function changed its name, forgetting two things:
that 1) it is irrelevant and 2) Thomas did the rename in question. ]
> >
> > And why is this fcking relevant ?
[ Thomas is really annoyed at Jaswinder writing wrong,
irrelevant, time wasting mails without showing any sign of
understanding the issues and without admitting fault. ]
> >
>
> I am dead sure, you really need some good manners and need to
> learn many things.
[ Jaswinder, still not admitting his fault, compounds his mistakes
by being even more condescending. ]
Jaswinder, this is really not an acceptable pattern of behavior on
lkml.
Ingo
Ingo,
> Jaswinder, this is really not an acceptable pattern of behavior on
> lkml.
>
Check last 50 emails from him. He is treating others like his slaves and
use bad words.
I was just trying to teach him how other feels when some one shout on
them without any reason.
If possible please also teach some manners to him and tell him to keep
his ugly mouth shut if he do not know how to speak with others.
I am surprise how you are accepting his unacceptable pattern.
--
JSR
* Jaswinder Singh Rajput <[email protected]> wrote:
> Ingo,
>
> > Jaswinder, this is really not an acceptable pattern of behavior
> > on lkml.
>
> Check last 50 emails from him. He is treating others like his
> slaves and use bad words.
If you have a problem with someone you should take that to private
mail or elsewhere. This is a mailing list for technical discussions
and the technical content of your mails here was wrong and incorrect
at every level.
Ingo
Commit-ID: 6ce51c431019310ca03371355a4366c4649fa349
Gitweb: http://git.kernel.org/tip/6ce51c431019310ca03371355a4366c4649fa349
Author: Luis Henriques <[email protected]>
AuthorDate: Wed, 1 Apr 2009 18:06:35 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 23 Apr 2009 08:45:48 +0200
genirq: do not execute DEBUG_SHIRQ when irq setup failed
When requesting an IRQ, the DEBUG_SHIRQ code executes a fake IRQ just to make
sure the driver is ready to receive an IRQ immediately. The problem was that
this fake IRQ was being executed even if interrupt line failed to be allocated
by __setup_irq.
Signed-off-by: Luis Henriques <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
[ fixed bug pointed out by a warning reported by Stephen Rothwell ]
Cc: Stephen Rothwell <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/irq/manage.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1516ab7..8c68d5b 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -768,7 +768,7 @@ int request_irq(unsigned int irq, irq_handler_t handler,
kfree(action);
#ifdef CONFIG_DEBUG_SHIRQ
- if (irqflags & IRQF_SHARED) {
+ if (!retval && (irqflags & IRQF_SHARED)) {
/*
* It's a shared IRQ -- the driver ought to be prepared for it
* to happen immediately, so let's make sure....