lkml.org 
[lkml]   [1999]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] SMP race fix [was Re: SMP lockup & 3c509 on 2.2.x [aka. the Deadly 'ping -f']]


On Thu, 6 May 1999, Andrea Arcangeli wrote:
>
> But in general it make not a lot of sense to disable an irq from the irq
> itself, because if you are the irq your are just assured by design that
> you can't reenter in yourself.

Yes. But the problem then becomes one of common code. It may make sense to
have code that wants to atomically make sure that it's running on only the
current CPU and that is called from different contexts (both irq and not),
and it just does a

disable_irq(myirq);
.. do some slow stuff ..
enable_irq(myirq);

and the dangerous thing is that it would work on UP, but mysteriously (and
nonobviously) lock up on SMP with your implementation.

I agree that the above is not necessarily _clever_ nor necessarily good
programming practice. But I think it's even worse programming practice if
we made it lock up with no messages for no really obvious reason, because
it _is_ at least straightforward and obvious code and as such we can't
really claim that it is outright evil.

So I fully agree with your point that there is a very subtle race in the
disable_irq() path. It's just that I want to think more about the
solution, and I'd ask you to think about it some more too.

For example, I don't think that changing meaning of the INPROGRESS bit the
way you did is the right approach. But the right approach _may_ be to
split the thing up into two bits: currently we kind of overload two
meanings on INPROGRESS ("BUSY" as in 'we're not taking it any more because
it didn't have a handler or it was disabled when it happened' and a
separate "RUNNING" as in 'we're right now in the process of handling that
particular IRQ').

That kind of split would make sense, and would allow the autoirq
behaviour, as well as noticing spurious interrupts with no handler (which
was a invaluable debugging tool for me where the current INPROGRESS
semantics made a lot of sense).

Alternatively, maybe there are other approaches - and whatever we do I do
believe that we need to handle the nesting case gracefully (your "nosync"
doesn't really help - because it isn't useful as a above kind of
exclusivity guarantee exactly because it doesn't wait, see?)

Linus


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:51    [W:1.529 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site