[PATCH 2/2] ne.c msleep not mdelay
David Fries
waiting for an interrupt, so timing isn't critical.
Signed-off-by: David Fries <da...@fries.net>
Cc: Atsushi Nemoto <an...@mba.ocn.ne.jp>
Cc: Paul Gortmaker <p_gor...@yahoo.com>
---
drivers/net/ne.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ne.c b/drivers/net/ne.c
index b0bf47e..bcbeff7 100644
--- a/drivers/net/ne.c
+++ b/drivers/net/ne.c
@@ -529,7 +529,7 @@ static int __init ne_probe1(struct net_device *dev, unsigned long ioaddr)
outb_p(0x00, ioaddr + EN0_RCNTLO);
outb_p(0x00, ioaddr + EN0_RCNTHI);
outb_p(E8390_RREAD+E8390_START, ioaddr); /* Trigger it... */
- mdelay(10); /* wait 10ms for interrupt to propagate */
+ msleep(10); /* wait 10ms for interrupt to propagate */
outb_p(0x00, ioaddr + EN0_IMR); /* Mask it again. */
dev->irq = probe_irq_off(cookie);
if (ei_debug > 2)
--
1.4.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Alan Cox
David Fries <da...@fries.net> wrote:
> mdelay(10) replaced by msleep(10) to give up the CPU, it's just
> waiting for an interrupt, so timing isn't critical.
It is too critical for a reschedule to occur.
NAK this one.
Atsushi Nemoto
> > mdelay(10) replaced by msleep(10) to give up the CPU, it's just
> > waiting for an interrupt, so timing isn't critical.
>
> It is too critical for a reschedule to occur.
>
> NAK this one.
There are already some msleep() in probe_irq_on() which is called just
before here. And this part is not protected by any spinlock or
preempt_disable.
So, if rescheduling was dangerous here, we already have potential
problems, no?
---
Atsushi Nemoto
David Fries
> On Sat, 30 Aug 2008 09:59:06 +0100, Alan Cox <al...@lxorguk.ukuu.org.uk> wrote:
> > > mdelay(10) replaced by msleep(10) to give up the CPU, it's just
> > > waiting for an interrupt, so timing isn't critical.
> >
> > It is too critical for a reschedule to occur.
> >
> > NAK this one.
>
> There are already some msleep() in probe_irq_on() which is called just
> before here. And this part is not protected by any spinlock or
> preempt_disable.
There is a probing_active mutex. probe_irq_off has the comment
"nothing prevents two IRQ probe callers from overlapping." Looks to me
like probing_active would prevent multiple probes from happening.
> So, if rescheduling was dangerous here, we already have potential
> problems, no?
I was just going to make the argument that any task that could be run
during msleep, could just as easily run on the other cpu in mdelay (if
there was one).
--
David Fries <da...@fries.net>
http://fries.net/~david/ (PGP encryption key available)
Alan Cox
> before here. And this part is not protected by any spinlock or
> preempt_disable.
>
> So, if rescheduling was dangerous here, we already have potential
> problems, no?
Yes we do but I didn't manage to stop the other mistakes getting in when
they did. If you take a schedule you get results back from the probe_irq
that tend to suck in other random ISA device events (ISA being edge
triggered nobody was ever careful about spurious irq)
Alan Cox
> during msleep, could just as easily run on the other cpu in mdelay (if
> there was one).
ISA bus auto-probing doesn't actually work at all well on SMP. The number
of people who have ever been affected by that has always been effectively
nil.
Atsushi Nemoto
> > So, if rescheduling was dangerous here, we already have potential
> > problems, no?
>
> Yes we do but I didn't manage to stop the other mistakes getting in when
> they did. If you take a schedule you get results back from the probe_irq
> that tend to suck in other random ISA device events (ISA being edge
> triggered nobody was ever careful about spurious irq)
OK, I see your point. Thanks.
---
Atsushi Nemoto