Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread John W. Linville
On Mon, Mar 27, 2006 at 10:39:46PM +0200, Roger Luethi wrote:
 On Fri, 24 Mar 2006 16:49:10 +0100, Marco Berizzi wrote:
  Hello evebody.
  I get this error on linux vanilla 2.6.16
  with via_rhine module loaded  when
  I run mii-tool:
 
 That was caused by a recent change that replaced an mdelay with msleep.
 
 netdev_ioctl and friends (ethtool calls, too) are known to grab a spin lock
 before they do much of anything, and they hang onto it until they're done.
 They also call mdio_read/write, which requires millisecond delays on Rhine-I.
 
 So on Rhine-I with a 2.6.15+ kernel, the driver ends up calling msleep
 while holding a spin lock -- hence the stack dump.
 
 I wonder if low latency for ancient Rhine-I chips is worth the trouble.

IIRC, the point was that mdelay was getting called in interrupt
context and causing ugly messages to show-up in dmesg.

Would the patch below be sufficient?  Or does the whole patch need
to be reverted?

John

diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 2418715..e7b4bc3 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -1145,8 +1130,8 @@ static void rhine_disable_linkmon(void _
if (quirks  rqRhineI) {
iowrite8(0x01, ioaddr + MIIRegAddr);// MII_BMSR
 
-   /* Do not call from ISR! */
-   msleep(1);
+   /* Can be called from ISR. Evil. */
+   mdelay(1);
 
/* 0x80 must be set immediately before turning it off */
iowrite8(0x80, ioaddr + MIICmd);
-- 
John W. Linville
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread Roger Luethi
On Thu, 13 Apr 2006 14:26:43 -0400, John W. Linville wrote:
  I wonder if low latency for ancient Rhine-I chips is worth the trouble.
 
 IIRC, the point was that mdelay was getting called in interrupt
 context and causing ugly messages to show-up in dmesg.

I suppose the patch back then was to reduce latency; the ugly messages in
the kernel ring buffer were _introduced_ with the patch (you shouldn't get 
error messages calling mdelay in interrupt context because that's what
mdelay is for).

 Would the patch below be sufficient?  Or does the whole patch need
 to be reverted?

I'd revert the whole thing. There's no point in having the additional
work_struct complexity if we end up calling mdelay anyway.

Roger
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread Roger Luethi
On Thu, 13 Apr 2006 11:40:18 -0700, Stephen Hemminger wrote:
 The right thing to do is get rid of the locking in via_rhine:netdev_ioctl
 and push the locking down into mdio_read, mdio_write.

As I said before, a dozen other network drivers do the exact same thing --
they call generic_mii_ioctl right after grabbing the private spin lock (and
only one driver calls generic_mii_ioctl without taking the lock).

I am not keen on patches that make via-rhine more of a special case even if
it was safe now; next thing you know generic_mii_ioctl is changed in a way
that breaks the only driver that foolishly made assumptions about the
side-effects of that function.

If you can safely move the locking down for all network drivers, that would
be a different story, of course.

Roger
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread Stephen Hemminger
On Thu, 13 Apr 2006 22:47:31 +0200
Roger Luethi [EMAIL PROTECTED] wrote:

 On Thu, 13 Apr 2006 11:40:18 -0700, Stephen Hemminger wrote:
  The right thing to do is get rid of the locking in via_rhine:netdev_ioctl
  and push the locking down into mdio_read, mdio_write.
 
 As I said before, a dozen other network drivers do the exact same thing --
 they call generic_mii_ioctl right after grabbing the private spin lock (and
 only one driver calls generic_mii_ioctl without taking the lock).
 

 I am not keen on patches that make via-rhine more of a special case even if
 it was safe now; next thing you know generic_mii_ioctl is changed in a way
 that breaks the only driver that foolishly made assumptions about the
 side-effects of that function.
 
 If you can safely move the locking down for all network drivers, that would
 be a different story, of course.
 
 Roger

Didn't your mother ever tell you that just because everybody else does
it wrong, you don't have to.

The other drivers should be fixed as well.  Phy access with irq's disabled
is not good. The hardware I checked takes 100's of usecs to do one read
transaction.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread Francois Romieu
Stephen Hemminger [EMAIL PROTECTED] :
[...]
 The other drivers should be fixed as well.  Phy access with irq's disabled
 is not good. The hardware I checked takes 100's of usecs to do one read
 transaction.

Yep, 802.3 allows it. :o|

It's surprizing that the low latency squadron has not complained so far
from our bad habit of spinlocked, irq disabled, MII access.

-- 
Ueimor
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-04-13 Thread Roger Luethi
On Thu, 13 Apr 2006 14:02:52 -0700, Stephen Hemminger wrote:
  I am not keen on patches that make via-rhine more of a special case even if
  it was safe now; next thing you know generic_mii_ioctl is changed in a way
  that breaks the only driver that foolishly made assumptions about the
  side-effects of that function.
  
  If you can safely move the locking down for all network drivers, that would
  be a different story, of course.
 
 Didn't your mother ever tell you that just because everybody else does
 it wrong, you don't have to.

No, but she warned me not to spend time on fixing botched low latency
patches.

Look, it took a lot of time to make via-rhine stable. It's still got
unexplained issues. I have a patch here for a bug that makes a driver
reload necessary when it occurs (and the patch is sitting here because
nobody's able to reproduce the problem anymore). I am lacking adequate
documentation, I have little time to work on the driver, but quite a
to do list.

Does that sound like via-rhine would make a good guinea pig?

 The other drivers should be fixed as well.  Phy access with irq's disabled
 is not good. The hardware I checked takes 100's of usecs to do one read
 transaction.

If you want to fix this in all drivers, more power to you. It is just not
high on my own priority list. I see the need for low latency, but latency
issues that only happen when people fiddle with MII settings don't seem all
that dramatic to me.

Roger
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: via_rhine modules error on 2.6.16 with mii-tool

2006-03-27 Thread Roger Luethi
On Fri, 24 Mar 2006 16:49:10 +0100, Marco Berizzi wrote:
 Hello evebody.
 I get this error on linux vanilla 2.6.16
 with via_rhine module loaded  when
 I run mii-tool:

That was caused by a recent change that replaced an mdelay with msleep.

netdev_ioctl and friends (ethtool calls, too) are known to grab a spin lock
before they do much of anything, and they hang onto it until they're done.
They also call mdio_read/write, which requires millisecond delays on Rhine-I.

So on Rhine-I with a 2.6.15+ kernel, the driver ends up calling msleep
while holding a spin lock -- hence the stack dump.

I wonder if low latency for ancient Rhine-I chips is worth the trouble.

Roger
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


via_rhine modules error on 2.6.16 with mii-tool

2006-03-24 Thread Marco Berizzi

Hello evebody.
I get this error on linux vanilla 2.6.16
with via_rhine module loaded  when
I run mii-tool:

cheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100] process_timeout+0x0/0x10
[c011c378] msleep+0x28/0x30
[c883d0bb] rhine_disable_linkmon+0xab/0x130 [via_rhine]
[c022f11f] do_con_write+0x22f/0x5a0
[c883d16f] mdio_read+0x2f/0xd0 [via_rhine]
[c88348c4] generic_mii_ioctl+0x84/0x164 [mii]
[c883e7b0] netdev_ioctl+0x0/0x80 [via_rhine]
[c883e7fd] netdev_ioctl+0x4d/0x80 [via_rhine]
[c025f764] dev_ifsioc+0x334/0x3a0
[c025f964] dev_ioctl+0x194/0x2a0
[c0254528] sock_ioctl+0xe8/0x1f0
[c01637de] do_ioctl+0x8e/0xa0
[c0163995] vfs_ioctl+0x65/0x1f0
[c0163b65] sys_ioctl+0x45/0x70
[c0102d55] syscall_call+0x7/0xb
scheduling while atomic: mii-tool/0x0001/923
[c02bfca2] schedule+0x632/0x640
[c02c0577] schedule_timeout+0x57/0xb0
[c011c100]