Hi Jens, Geert,
> > >
> > > If you look at the code long enough, you will notioce that the
> > > local_irq_disable() call is actually commented out. This has been
> > > introduced back in 2002 in [1], but as you can see, the same bug has been
> > > there even before, with the sti() call being commented out in the very
> > > same way :)
Please note above: the commented out line used to be sti() - this is still
present in 2.4.30 which I've been running for a long time. The driver uses
timers for various tasks, and this was to ensure timer interrupts are on during
redo_fd_request.
The change in 2002 substituted local_irq_disable() for the sti() which seems
clearly wrong.
> > I would rather suggest to leave the code in, and fix the buggy
> > comments instead.
>
> What buggy comments? The comments states that interrupts are already
> disabled when entering this function, which is correct. The point is
> that doing a flags save and then an irq disable is pointless, since we
> KNOW that interrupts are already disabled.
That's what is worrying me - enabling interrupts for the sake of getting timers
working used to be necessary at one time.
There is something else wrong with do_fd_request - if indeed interrupts are
disabled, stdma_lock may attempt to sleep and would deadlock.
I guess scheduling redo_fd_request as bottom half or delayed work would be
required here. I need to look into this more closely.
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
