Dear Philip, Linus, Alan,
one more jiffies cleanup patch to use comparison macros as pointed out by
Andreas Dilger on lkml.
I try to bundle files with the same maintainer to reduce email overhead.
If my procedure of mailing the maintainer as well as Cc:ing Linus and Alan
is incorrect, please drop me a short note.
In eexpress.c I also turned absolute jiffies number into multiples of HZ,
yet the resulting timeout values still do not always seem reasonable to
me.
Tim
--- ../linux-2.4.14-pre6/drivers/char/lp.c Wed Oct 31 23:06:19 2001
+++ drivers/char/lp.c Tue Nov 6 20:37:30 2001
@@ -302,7 +302,7 @@
size_t copy_size = count;
#ifdef LP_STATS
- if (jiffies-lp_table[minor].lastcall > LP_TIME(minor))
+ if (time_after(jiffies, lp_table[minor].lastcall + LP_TIME(minor)))
lp_table[minor].runchars = 0;
lp_table[minor].lastcall = jiffies;
--- ../linux-2.4.14-pre6/drivers/net/eexpress.c Sun Sep 30 21:26:06 2001
+++ drivers/net/eexpress.c Tue Nov 6 20:52:59 2001
@@ -504,7 +504,7 @@
if (lp->started)
{
- if ((jiffies - dev->trans_start)>50)
+ if (time_after(jiffies, dev->trans_start + HZ/2))
{
if (lp->tx_link==lp->last_tx_restart)
{
@@ -560,7 +560,7 @@
}
else
{
- if ((jiffies-lp->init_time)>10)
+ if (time_after(jiffies, lp->init_time + HZ/10))
{
unsigned short status = scb_status(dev);
printk(KERN_WARNING "%s: i82586 startup timed out, status %04x, resetting...\n",
@@ -725,8 +725,8 @@
static void eexp_cmd_clear(struct net_device *dev)
{
- unsigned long int oldtime = jiffies;
- while (scb_rdcmd(dev) && ((jiffies-oldtime)<10));
+ unsigned long timeout = jiffies + HZ/10;
+ while (scb_rdcmd(dev) && (time_before(jiffies, timeout)));
if (scb_rdcmd(dev)) {
printk("%s: command didn't clear\n", dev->name);
}
@@ -1598,16 +1598,16 @@
}
}
if (kick) {
- unsigned long oj;
+ unsigned long timeout;
scb_command(dev, SCB_CUsuspend);
outb(0, ioaddr+SIGNAL_CA);
outb(0, ioaddr+SIGNAL_CA);
#if 0
printk("%s: waiting for CU to go suspended\n", dev->name);
#endif
- oj = jiffies;
+ timeout = jiffies + 20*HZ;
while ((SCB_CUstat(scb_status(dev)) == 2) &&
- ((jiffies-oj) < 2000));
+ time_before(jiffies, timeout));
if (SCB_CUstat(scb_status(dev)) == 2)
printk("%s: warning, CU didn't stop\n", dev->name);
lp->started &= ~(STARTED_CU);
On Nov 06, 2001 21:04 +0100, Tim Schmielau wrote:
> In eexpress.c I also turned absolute jiffies number into multiples of HZ,
> yet the resulting timeout values still do not always seem reasonable to
> me.
I agree. It seems very ugly. I looked at a few drivers which loop 1 or 2
jiffies, but to busy-loop for 1/10th of a second, or even 20 seconds
is terribly bad. I took a look at the functions scb_rdcmd() and
scb_status() and they are only reading from an I/O port, so no chance
of sleeping/blocking, just busy waiting.
> - unsigned long int oldtime = jiffies;
> - while (scb_rdcmd(dev) && ((jiffies-oldtime)<10));
> + unsigned long timeout = jiffies + HZ/10;
> + while (scb_rdcmd(dev) && (time_before(jiffies, timeout)));
> if (scb_rdcmd(dev)) {
> printk("%s: command didn't clear\n", dev->name);
> }
> @@ -1598,16 +1598,16 @@
> #endif
> - oj = jiffies;
> + timeout = jiffies + 20*HZ;
> while ((SCB_CUstat(scb_status(dev)) == 2) &&
> - ((jiffies-oj) < 2000));
> + time_before(jiffies, timeout));
> if (SCB_CUstat(scb_status(dev)) == 2)
> printk("%s: warning, CU didn't stop\n", dev->name);
> lp->started &= ~(STARTED_CU);
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/
In message <[email protected]>, Andreas Dilger writes:
>On Nov 06, 2001 21:04 +0100, Tim Schmielau wrote:
>> In eexpress.c I also turned absolute jiffies number into multiples of HZ,
>> yet the resulting timeout values still do not always seem reasonable to
>> me.
>
>I agree. It seems very ugly. I looked at a few drivers which loop 1 or 2
>jiffies, but to busy-loop for 1/10th of a second, or even 20 seconds
>is terribly bad.
Those timeouts are only a last resort. If the card is working properly the loop will terminate much sooner.
p.
On Nov 06, 2001 21:37 +0000, Philip Blundell wrote:
> In message <[email protected]>, Andreas Dilger writes:
> >I agree. It seems very ugly. I looked at a few drivers which loop 1 or 2
> >jiffies, but to busy-loop for 1/10th of a second, or even 20 seconds
> >is terribly bad.
>
> Those timeouts are only a last resort. If the card is working properly
> the loop will terminate much sooner.
Well, if the card is working properly, then you don't need the timeouts
at all. Clearly, if they are needed, then either they should be more
realistic in length (1/10th isn't bad, but 20 seconds?), or after a
"reasonable" short timeout, it should sleep for an interval and check
afterwards. Sucking all CPU for 20 seconds and locking everything else
out isn't an acceptable method IMHO.
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/
> afterwards. Sucking all CPU for 20 seconds and locking everything else
> out isn't an acceptable method IMHO.
There are drivers that do this WITH INTERRUPTS OFF. Some systems (e.g.
Cobalt) have short-interval watchdog timers, and when someone uses one of
these drivers, they just see spontaneous reboots. Ick.