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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
