Hi Matt,
Here's a couple small tweaks. The first is to netpoll_setup. The settle
time was too short for my e100, and the system would hang. The second
is to netconsole so that it registers a console with CON_PRINTBUFFER.
This helps debugging early bootup issues where you want to capture data
from before netconsole is initialized. Perhaps it should be a param
to netconsole?
thanks,
-chris
--- 2.6.0-test5-mm2/net/core/netpoll.c.wait_fix 2003-09-15 15:46:28.000000000 -0700
+++ 2.6.0-test5-mm2/net/core/netpoll.c 2003-09-15 16:14:20.000000000 -0700
@@ -526,7 +526,7 @@ int netpoll_setup(struct netpoll *np)
rtnl_shunlock();
/* Give driver a chance to settle */
- jiff = jiffies + 2*HZ;
+ jiff = jiffies + 4*HZ;
while (time_before(jiffies, jiff))
;
}
--- 2.6.0-test5-mm2/drivers/net/netconsole.c.print_buf 2003-09-15 16:21:31.000000000 -0700
+++ 2.6.0-test5-mm2/drivers/net/netconsole.c 2003-09-17 11:09:59.000000000 -0700
@@ -95,7 +95,7 @@
}
static struct console netconsole = {
- .flags = CON_ENABLED,
+ .flags = CON_ENABLED | CON_PRINTBUFFER,
.write = write_msg
};
On Wed, Sep 17, 2003 at 11:24:47AM -0700, Chris Wright wrote:
> Hi Matt,
>
> Here's a couple small tweaks. The first is to netpoll_setup. The settle
> time was too short for my e100, and the system would hang.
This probably ought to be a command line arg, my tg3 apparently
doesn't need it at all. One second is what's in the nfs-root stuff,
perhaps they need to share an arg, I changed it to two because my tlan
was dropping stuff (but not hanging - that may be a driver bug).
Ideally, I'd like to find a way to wait until the damn thing is up
before sending.
net/ipv4/ipconfig.c:
/* Define the friendly delay before and after opening net devices */
#define CONF_PRE_OPEN (HZ/2) /* Before opening: 1/2 second */
#define CONF_POST_OPEN (1*HZ) /* After opening: 1 second */
Anyway, I'm still struggling with getting stuff working on my Opteron
box, care to take a stab at it?
> The second
> is to netconsole so that it registers a console with CON_PRINTBUFFER.
> This helps debugging early bootup issues where you want to capture data
> from before netconsole is initialized. Perhaps it should be a param
> to netconsole?
I think this can probably be unconditional. Merged.
--
Matt Mackall : http://www.selenic.com : of or relating to the moon
* Matt Mackall ([email protected]) wrote:
> On Wed, Sep 17, 2003 at 11:24:47AM -0700, Chris Wright wrote:
> > Here's a couple small tweaks. The first is to netpoll_setup. The settle
> > time was too short for my e100, and the system would hang.
>
> This probably ought to be a command line arg, my tg3 apparently
> doesn't need it at all. One second is what's in the nfs-root stuff,
> perhaps they need to share an arg, I changed it to two because my tlan
> was dropping stuff (but not hanging - that may be a driver bug).
> Ideally, I'd like to find a way to wait until the damn thing is up
> before sending.
Yeah, I agree, that seems best.
> net/ipv4/ipconfig.c:
> /* Define the friendly delay before and after opening net devices */
> #define CONF_PRE_OPEN (HZ/2) /* Before opening: 1/2 second */
> #define CONF_POST_OPEN (1*HZ) /* After opening: 1 second */
>
> Anyway, I'm still struggling with getting stuff working on my Opteron
> box, care to take a stab at it?
Sure, I'll poke at it tomorrow, unless you get to it first.
> > The second
> > is to netconsole so that it registers a console with CON_PRINTBUFFER.
> > This helps debugging early bootup issues where you want to capture data
> > from before netconsole is initialized. Perhaps it should be a param
> > to netconsole?
>
> I think this can probably be unconditional. Merged.
Ok, cool.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Mer, 2003-09-17 at 19:24, Chris Wright wrote:
> /* Give driver a chance to settle */
> - jiff = jiffies + 2*HZ;
> + jiff = jiffies + 4*HZ;
> while (time_before(jiffies, jiff))
<pedant>
> ;
should be cpu_relax();
</pedant>
* Alan Cox ([email protected]) wrote:
> On Mer, 2003-09-17 at 19:24, Chris Wright wrote:
> > /* Give driver a chance to settle */
> > - jiff = jiffies + 2*HZ;
> > + jiff = jiffies + 4*HZ;
> > while (time_before(jiffies, jiff))
> <pedant>
> should be cpu_relax();
> </pedant>
Hrm, there's many spots that aren't using it. What's the benefit, less
power consumption? Is it worth a patch to convert other things over?
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
> Hrm, there's many spots that aren't using it. What's the benefit, less
> power consumption?
Less power consumption, and on HT/SMT CPU's it's a "yield" to the other
half/halves.
>
> Is it worth a patch to convert other things over?
>
yes
* Arjan van de Ven ([email protected]) wrote:
> Less power consumption, and on HT/SMT CPU's it's a "yield" to the other
> half/halves.
I see, thanks.
> > Is it worth a patch to convert other things over?
> yes
OK, I'll spin one up, thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
On Iau, 2003-09-18 at 17:48, Chris Wright wrote:
> > <pedant>
> > should be cpu_relax();
> > </pedant>
>
> Hrm, there's many spots that aren't using it. What's the benefit, less
> power consumption? Is it worth a patch to convert other things over?
It speeds up hyperthreading CPUs by letting them know that this
paticular thread is waiting for stuff (sched_yield for silicon)
On Thu, 18 Sep 2003, Alan Cox wrote:
> On Iau, 2003-09-18 at 17:48, Chris Wright wrote:
> > > <pedant>
> > > should be cpu_relax();
> > > </pedant>
> >
> > Hrm, there's many spots that aren't using it. What's the benefit, less
> > power consumption? Is it worth a patch to convert other things over?
>
> It speeds up hyperthreading CPUs by letting them know that this
> paticular thread is waiting for stuff (sched_yield for silicon)
>
Does that mean that it would be benneficial to do something like this in
for example eepro100.c ??
diff -up linux-2.6.0-test5-orig/drivers/net/eepro100.c
linux-2.6.0-test5/drivers/net/eepro100.c
--- linux-2.6.0-test5-orig/drivers/net/eepro100.c 2003-09-08 21:50:09.000000000 +0200
+++ linux-2.6.0-test5/drivers/net/eepro100.c 2003-09-19 01:03:19.000000000 +0200
@@ -913,9 +913,11 @@ static void do_slow_command(struct net_d
for (wait = 0; wait <= 100; wait++)
if (inb(cmd_ioaddr) == 0) return;
- for (; wait <= 20000; wait++)
+ for (; wait <= 20000; wait++) {
if (inb(cmd_ioaddr) == 0) return;
else udelay(1);
+ cpu_relax();
+ }
printk(KERN_ERR "Command %4.4x was not accepted after %d polls!"
" Current status %8.8x.\n",
cmd, wait, inl(dev->base_addr + SCBStatus));
or maybe even take it to the extreme like
diff -up linux-2.6.0-test5-orig/drivers/net/eepro100.c
linux-2.6.0-test5/drivers/net/eepro100.c
--- linux-2.6.0-test5-orig/drivers/net/eepro100.c 2003-09-08 21:50:09.000000000 +0200
+++ linux-2.6.0-test5/drivers/net/eepro100.c 2003-09-19 01:07:50.000000000 +0200
@@ -902,20 +902,25 @@ static void do_slow_command(struct net_d
{
long cmd_ioaddr = dev->base_addr + SCBCmd;
int wait = 0;
- do
+ do {
if (inb(cmd_ioaddr) == 0) break;
- while(++wait <= 200);
+ cpu_relax();
+ } while(++wait <= 200);
if (wait > 100)
printk(KERN_ERR "Command %4.4x never accepted (%d
polls)!\n",
inb(cmd_ioaddr), wait);
outb(cmd, cmd_ioaddr);
- for (wait = 0; wait <= 100; wait++)
+ for (wait = 0; wait <= 100; wait++) {
if (inb(cmd_ioaddr) == 0) return;
- for (; wait <= 20000; wait++)
+ cpu_relax();
+ }
+ for (; wait <= 20000; wait++) {
if (inb(cmd_ioaddr) == 0) return;
else udelay(1);
+ cpu_relax();
+ }
printk(KERN_ERR "Command %4.4x was not accepted after %d polls!"
" Current status %8.8x.\n",
cmd, wait, inl(dev->base_addr + SCBStatus));
How short/long waits are we talking about before it is benneficial to be
calling cpu_relax() ?
Kind regards,
Jesper Juhl <[email protected]>
On Fri, 2003-09-19 at 01:10, Jesper Juhl wrote:
> 00; wait++)
> if (inb(cmd_ioaddr) == 0) return;
> - for (; wait <= 20000; wait++)
> + for (; wait <= 20000; wait++) {
> if (inb(cmd_ioaddr) == 0) return;
> else udelay(1);
> + cpu_relax();
> + }
udelay() should have cpu_relax() inside it already basically..