mdelay(10) replaced by msleep(10) to give up the CPU, it's just
waiting for an interrupt, so timing isn't critical.
Signed-off-by: David Fries <[email protected]>
Cc: Atsushi Nemoto <[email protected]>
Cc: Paul Gortmaker <[email protected]>
---
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
On Fri, 29 Aug 2008 21:44:53 -0500
David Fries <[email protected]> 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.
On Sat, 30 Aug 2008 09:59:06 +0100, Alan Cox <[email protected]> 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.
So, if rescheduling was dangerous here, we already have potential
problems, no?
---
Atsushi Nemoto
On Sat, Aug 30, 2008 at 11:36:47PM +0900, Atsushi Nemoto wrote:
> On Sat, 30 Aug 2008 09:59:06 +0100, Alan Cox <[email protected]> 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 <[email protected]>
http://fries.net/~david/ (PGP encryption key available)
> 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?
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)
> 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).
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.
On Sat, 30 Aug 2008 15:44:47 +0100, Alan Cox <[email protected]> wrote:
> > 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