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

List:       linux-arm-kernel
Subject:    Re: race condition in do_IRQ
From:       Matthias Welwarsky <mwelwarsky () web ! de>
Date:       2004-05-06 16:42:20
Message-ID: 200405061842.20760.mwelwarsky () web ! de
[Download RAW message or body]

On Thursday 06 May 2004 18:03, Marc Singer wrote:
> On Thu, May 06, 2004 at 11:56:38AM +0200, Marius Groeger wrote:
> > Robin,
> >
> > On Thu, 6 May 2004, Robin Farine wrote:
> > > I may be wrong but here is how I understand it. This part of the code
> > > runs with the irq_controller_lock held and interrupts globally
> > > disabled. For the duration of each interrupt handler, the common
> > > interrupt handling code releases the lock (and enables interrupt iff
> > > the handler has not specified the SA_INTERRUPT flag). Thus, this code
> > > respects atomicity with respect to nested interrupts and SMP.
> > >
> > > While a handler is executing with interrupts enabled, a new interrupt
> > > routed to the same vector may occur. The running flag prevents nested
> > > executions of the associated handler.
> >
> > Thank you and Matthias for clearing this up. May I humbly suggest to
> > insert some text along these lines as comment?
>
> While it might look like a good idea to insert such a comment, it can
> introduce it's own problems.  1) Comments let people off of the hook
> from reading the code.  2) They may be wrong which leads to confusion
> and some people misunderstanding the code since they read the
> incorrect comments instead of the code.
>
> We're all better off reading the code than reading comments.  Really.

I second that as far as comments must not be excuse for unreadable code. 
However, code is always condensed thought, and getting the idea behind tricky 
code is most of the times far from obvious, no matter how well the code is 
written.

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 
that looks completely wrong, if you're not familiar with it, assume the 
p.e.b.k.a.c ;)

regards,
	matthias

-------------------------------------------------------------------
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]