[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-arm-kernel
Subject:    Re: race condition in do_IRQ
From:       Russell King - ARM Linux <linux () arm ! linux ! org ! uk>
Date:       2004-05-07 8:37:52
Message-ID: 20040507093752.A15722 () flint ! arm ! linux ! org ! uk
[Download RAW message or body]

On Fri, May 07, 2004 at 10:15:25AM +0200, Marius Groeger wrote:
> On Thu, 6 May 2004, Matthias Welwarsky wrote:
> > However, in this particular case, the simple fact that this code is in use in
> > production systems for some time should have lead to the assumption that it
> > cannot be _that_ borked. Rule of thumb: if you find code in the linux kernel
> 
> To explain the background: we are trying to find a tricky bug with current
> RTAI on ARM7 and need to understand the Linux side completely first.

Doesn't that mean that you also need to understand the ARM interrupt
vector, the assembly from that point onwards, the C code in asm_do_IRQ
and how that calls the IRQ descriptor handler - which means that you
gain the knowledge you were missing in the first place.

Of course, if you jump into the middle of some random code you'll never
know the conditions under which it's called.

> On Thu, 6 May 2004, Marc Singer wrote:
> > We're all better off reading the code than reading comments.  Really.
> 
> If a function relies on lockings that happen outside it's scope, this does
> deserve a comment, don't you think?

Do the interrupt handlers themselves have this commenting?  No.  Do the
filesystem functions document these conditions?  No.  Does the MM code
document this?  No.  Does arch/i386/kernel/irq.c::do_IRQ document the
conditions under which it's called?  No.  Does it document the conditions
under which it's handler function pointers are called?  No.

Do you want me to continue giving examples?  Probably not. 8)

> The same chapter also demands to write code which is obvious. I don't feel
> that the code in question lives up to this, but of course, YMMV. If the code
> works on the implicit assumption that a certain lock is in effect, this is
> not obvious or readable from this code in the slightest way.

Sure, if it was and it was designed to be _called_ by external code
then you'd be correct, but it isn't.  It's called from a well-defined
path through the code and that path defines its preconditions.

The only reason it is global is that its value is designed to be used
with set_irq_handler() and nothing else.

-------------------------------------------------------------------
Subscription options: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:       http://www.arm.linux.org.uk/armlinux/mlfaq.php
Etiquette: http://www.arm.linux.org.uk/armlinux/mletiquette.php
[prev in list] [next in list] [prev in thread] [next in thread]