PROBLEM: Interrupt Handling Code (handle_IRQ_event)

PROBLEM: Interrupt Handling Code (handle_IRQ_event)

[Gaurav Dhiman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=us-ascii, Size: 3685 bytes --]


Hi All,

It’s a Bug-Report.
I found a bug in interrupt handling code of Kernel. I
think it is probably a bug.
It’s not a bug, which I encountered while running a
linux box, so I can not provide the output of bug or
how to re-produce it (as suggested in most of the
bug-reporting articles).

I have been analyzing the interrupt handling code of
kernel for i386 platform and found the following,
which I think might be a bug, but at the same time I
also think that kernel developers who actually coded
that segment of kernel code are the right people to
tell me if it’s a bug or not.

Following is the description of it:

- In handle_IRQ_event() function, we are actually
checking the flag of only the first handler for IRQ
line (see the code at :
http://lxr.linux.no/source/arch/i386/kernel/irq.c?v=2.4.21#L430).
Shouldn't we check the flag for each handler
registered for particular IRQ, before invoking that
handler ? As handlers are registered by different
device drivers which can share the IRQ line, they can
be registered as of different types (slow and fast
handlers) for same IRQ.

In 2.4.21 version of kernel, the bug related code in
handle_IRQ_event() function looks like this

	if (!(action->flags & SA_INTERRUPT))
		__sti();

	do {
		status |= action->flags;
		action->handler(irq, action->dev_id, regs);
		action = action->next;
	} while (action); 


If we take the following scenario, above code will not
work as it should work:
- if the first handler in handlers queue for an IRQ is
of slow interrupt type (making the interrupts enable)
and there is any other handler further in the queue
whose flag says that it’s a fast handler (should be
called with interrupt disabled), then in that case,
while calling the first handler, above code will
enable the interrupts and when the fast interrupt
handler further in queue will be called the interrupts
will still be enabled, where as it should be disabled
while calling a fast interrupt.

I think the solution of this would be that, before
calling a handler, we should check the flag of that
handler. If flag says it’s a fast handler, we should
disable the interrupts using __cli(), but if the flag
says it's a slow handler, we should enable interrupts
using __sti() function.

I think the correct code should be somewhat like the
following one:

	do {
		if (!(action->flags & SA_INTERRUPT))
			__sti();
		else
			__cli();

		status |= action->flags;
		action->handler(irq, action->dev_id, regs);
		action = action->next;
	} while (action);

I even checked the latest kernel version 2.6.8.1,
things are same there, so if it is a bug, we need to
fix it. I don’t know if this bug is harmful in some
manner or just a sleeping bug, which does not affect
the kernel functionality.

I think for resolving this problem, my machine
information will be irrelevant as I did not
encountered this bug while running my linux machine,
so I am not sending the machine specific information.
If you still need it, please do let me know, I will
provide it.

Please comment on this, what I observed is a bug or
not ? If it is a bug, please do let me know how to
take the responsibility of bug-fixing as I would like
to provide the fix for it.

At last, please do let me know one more thing, how can
I track the chain of mails for this bug on
linux-kernel@vger.kernel.org without subscribing it. I
have not subscribed to this mailing list as I know
it’s a heavy mailing list. Please let me know the way
to track all replies to this mail without subscribing
the kernel mailing list.
 
Regards,
Gaurav



		
__________________________________
Do you Yahoo!?
New and Improved Yahoo! Mail - Send 10MB messages!
http://promotions.yahoo.com/new_mail 

Re: PROBLEM: Interrupt Handling Code (handle_IRQ_event)

[Dhiman, Gaurav]

-----Original Message-----
From: Gaurav Dhiman [mailto:gauravd_linux@yahoo.com] 
Sent: Friday, September 24, 2004 10:24 PM
To: Dhiman, Gaurav
Subject: Fwd: Re: PROBLEM: Interrupt Handling Code (handle_IRQ_event)


--- ar.karthick@veritas.com wrote:

> Date: Fri, 24 Sep 2004 20:58:51 +0530 (IST)
> From: ar.karthick@veritas.com
> To: Gaurav Dhiman <gauravd_linux@yahoo.com>
> Subject: Re: PROBLEM: Interrupt Handling Code
> (handle_IRQ_event)
> 
> >
> > Hi All,
> >
> > It's a Bug-Report.
> > I found a bug in interrupt handling code of
> Kernel. I
> > think it is probably a bug.
> > It's not a bug, which I encountered while running
> a
> > linux box, so I can not provide the output of bug
> or
> > how to re-produce it (as suggested in most of the
> > bug-reporting articles).
> > 	do {
> > 		if (!(action->flags & SA_INTERRUPT))
> >                      __sti();
> > 		else
> > 			__cli();
> >
> > 		status |= action->flags;
> > 		action->handler(irq, action->dev_id, regs);
> > 		action = action->next;
> > 	} while (action);
> >
>   I dont think its a _bug_.
>   Its a win for a non-shared IRQ line.
>   You can get an IRQ for only 1 driver at a time.
>   But the handle_IRQ_event doesnt know the intended
> destination or
>   the action->handler.
>   So for a shared IRQ line, it tries calling all the
> registered IRQ 
> handlers 1 at a time, and its the job of the
> individual handlers to see if 
> its intended for them.
>   Your fix would add an additional overhead on the
> fast-path by
>   checking each action handler to disable/enable
> interrupt line
>   even though that handler wouldnt be actually
> handling the interrupt line.
>   You will end up doing redundant cli/stis on all
> the action->handler, 
> even though only 1 guy is going to handle the
> interrupt.
>   Its an overhead incurred on the fast-path which is
> a waste of 
> time,considering that you actually dont know till
> the "handler" returns, 
> as to who has handled the IRQ.
> 
>   Regards,
>   -Karthick
> -- 
> A.R.Karthick
> Veritas Software,
> Bund Garden Road,
> Next to Ruby Hall Clinic,
> Pune-411001
> India
> Tel: Office: 020 - 406 6905 (Direct)
> Home: http://mir-os.sourceforge.net
> 


I don't think it's a right way to handle interrupts. You are saying that
we should allow the handler to decide if it wants to disable the
interrupts or not, but then can you tell me the use of telling kernel
about our handler type (slow or fast) while registering our handler to
kernel using flag parameter in request_irq() function.

I think the main purpose of passing this flag parameter (third
parameter) in request_irq() function is that the kernel code can disable
or enable the interrupts (as per our flag passed) before invoking our
handler. If it's an handler itself who has to decide, then there is no
use of passing this flag parameter.

Moreover if it is the case as you stated then we should not even check
the type of first handler, which we are doing in current kernel code.
First handler should also be able to decide if it wants the interrupts
to be enabled or disabled according to its own requirement.

I raised the same issue on kernelnewbies mailing list also and got
majority of responses that it might be a bug. Some guys told that it
might be a left over bug, as in earlier versions kernel used to allow
only one type of handlers to share an IRQ line, so at that time only
checking the type of first handler used to do our work (as handlers used
to be of same type), but now as different types of handlers can share a
same IRQ line, so we need to check the type of handler before invoking
it.

Moreover as explained in Interrupt handling chapter of Linux Device
Driver (LDD) book (http://www.xml.com/ldd/chapter/book/ch09.html#t3)
under section "The internals of interrupt handling on the x86", it
handle_IRQ_event() function which actually take care of enabling and
disabling the interrupts rather than the handler itself. Following is
the text from LDD book.

"The function handle_IRQ_event is called to actually invoke the
handlers. It starts by testing a global interrupt lock bit; if that bit
is set, the processor will spin until it is cleared. Calling cli sets
this bit, thus blocking handling of interrupts; the normal interrupt
handling mechanism does not set this bit, and thus allows further
processing of interrupts. If the handler is of the slow variety,
interrupts are reenabled in the hardware and the handler is invoked.
Then it's just a matter of cleaning up, running tasklets and bottom
halves, and getting back to regular work"


Regards,
Gaurav Dhiman.
_____________________________
Thanks & Regards,
Gaurav Dhiman
Computer Associates - ITC,
eTrust Security Solutions
Hyderabad, India
Mobile: +91-040-32384853
Direct Tel: +91-40-55670911
Website: www.ca.com
E-Mail: Gaurav.Dhiman@ca.com
_____________________________