* [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace
2004-09-15 18:18 [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace Nishanth Aravamudan
@ 2004-09-15 18:20 ` Nishanth Aravamudan
2004-09-16 7:06 ` Nishanth Aravamudan
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nishanth Aravamudan @ 2004-09-15 18:20 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]
Sorry, wrong subject line. Any comments would be appreciated.
Description: msleep_interruptible() is used instead of schedule_timeout()
to guarantee the task delays as expected. To achieve this, the timeout
variable is converted from an int to an unsigned long, and it's values
are changed accordingly.
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
--- 2.6.9-rc2-vanilla/drivers/atm/idt77252.c 2004-09-13 17:16:06.000000000 -0700
+++ 2.6.9-rc2/drivers/atm/idt77252.c 2004-09-14 11:34:25.000000000 -0700
@@ -2516,7 +2516,7 @@ idt77252_close(struct atm_vcc *vcc)
struct vc_map *vc = vcc->dev_data;
unsigned long flags;
unsigned long addr;
- int timeout;
+ unsigned long timeout;
down(&card->mutex);
@@ -2566,9 +2566,9 @@ done:
}
spin_unlock_irqrestore(&vc->lock, flags);
- timeout = 5 * HZ;
+ timeout = 5 * 1000;
while (atomic_read(&vc->scq->used) > 0) {
- timeout = schedule_timeout(timeout);
+ timeout = msleep_interruptible(timeout);
if (!timeout)
break;
}
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace
2004-09-15 18:18 [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace Nishanth Aravamudan
2004-09-15 18:20 ` Nishanth Aravamudan
@ 2004-09-16 7:06 ` Nishanth Aravamudan
2004-09-16 14:46 ` Randy.Dunlap
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nishanth Aravamudan @ 2004-09-16 7:06 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]
On Thu, Sep 16, 2004 at 09:13:30AM +0200, Alexander Nyberg wrote:
> On Wed, 2004-09-15 at 20:20, Nishanth Aravamudan wrote:
> > Sorry, wrong subject line. Any comments would be appreciated.
> >
> > Description: msleep_interruptible() is used instead of schedule_timeout()
> > to guarantee the task delays as expected. To achieve this, the timeout
> > variable is converted from an int to an unsigned long, and it's values
> > are changed accordingly.
> >
> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> >
> >
> > --- 2.6.9-rc2-vanilla/drivers/atm/idt77252.c 2004-09-13 17:16:06.000000000 -0700
> > +++ 2.6.9-rc2/drivers/atm/idt77252.c 2004-09-14 11:34:25.000000000 -0700
> > @@ -2516,7 +2516,7 @@ idt77252_close(struct atm_vcc *vcc)
> > struct vc_map *vc = vcc->dev_data;
> > unsigned long flags;
> > unsigned long addr;
> > - int timeout;
> > + unsigned long timeout;
> >
> > down(&card->mutex);
> >
> > @@ -2566,9 +2566,9 @@ done:
> > }
> > spin_unlock_irqrestore(&vc->lock, flags);
> >
> > - timeout = 5 * HZ;
> > + timeout = 5 * 1000;
> > while (atomic_read(&vc->scq->used) > 0) {
> > - timeout = schedule_timeout(timeout);
> > + timeout = msleep_interruptible(timeout);
> > if (!timeout)
> > break;
> > }
> >
> > ______________________________________________________________________
> Please do not assume that HZ == 1000, it varies for different
> architectures.
No such assumption is being made. The former code:
timeout = 5 * HZ;
translates to "I want to sleep for 5 * HZ jiffies." HZ jiffies is just
the number of jiffies in 1 second, so this is equivalent to "I want to
sleep for 5 seconds."
Similarly, in the new code:
timeout = 5 * 1000;
translates to "I want to sleep for 5 * 1000 msecs." 1000 msecs is
clearly just the number of msecs in 1 second, so this is *also*
equivalent to "I want to sleep for 5 seconds," provided that msleep* is
used instead of schedule_timeout().
-Nish
P.S. Please keep replies on-list.
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace
2004-09-15 18:18 [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace Nishanth Aravamudan
2004-09-15 18:20 ` Nishanth Aravamudan
2004-09-16 7:06 ` Nishanth Aravamudan
@ 2004-09-16 14:46 ` Randy.Dunlap
2004-09-16 14:57 ` Greg KH
2004-09-16 15:27 ` Randy.Dunlap
4 siblings, 0 replies; 6+ messages in thread
From: Randy.Dunlap @ 2004-09-16 14:46 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]
[This is from Walter Harms, but several of his messages
don't make it thru the email system for some reason... --Randy]
HZ may vary between different architectures. was that the autors real
intend ?
- timeout = 5 * HZ;
+ timeout = 5 * 1000;
mfg
wh
[Randy says:] I agree with Walter. That change looks suspicious
or incorrect.
- - - - - - - - - - - - - - Original Message - - - - - - - - - - - - - -
From: Nishanth Aravamudan <nacc@us.ibm.com>
Subject: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace schedule_ti
Date: 09/15/04 20:18
Any comments would be appreciated.
Description: msleep_interruptible() is used instead of schedule_timeout()
to guarantee the task delays as expected. To achieve this, the timeout
variable is converted from an int to an unsigned long, and it's values
are changed accordingly.
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
--- 2.6.9-rc2-vanilla/drivers/atm/idt77252.c 2004-09-13 17:16:06.000000000 -0700
+++ 2.6.9-rc2/drivers/atm/idt77252.c 2004-09-14 11:34:25.000000000 -0700
@@ -2516,7 +2516,7 @@ idt77252_close(struct atm_vcc *vcc)
struct vc_map *vc = vcc->dev_data;
unsigned long flags;
unsigned long addr;
- int timeout;
+ unsigned long timeout;
down(&card->mutex);
@@ -2566,9 +2566,9 @@ done:
}
spin_unlock_irqrestore(&vc->lock, flags);
- timeout = 5 * HZ;
+ timeout = 5 * 1000;
while (atomic_read(&vc->scq->used) > 0) {
- timeout = schedule_timeout(timeout);
+ timeout = msleep_interruptible(timeout);
if (!timeout)
break;
}
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace
2004-09-15 18:18 [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace Nishanth Aravamudan
` (2 preceding siblings ...)
2004-09-16 14:46 ` Randy.Dunlap
@ 2004-09-16 14:57 ` Greg KH
2004-09-16 15:27 ` Randy.Dunlap
4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2004-09-16 14:57 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 286 bytes --]
On Thu, Sep 16, 2004 at 07:46:50AM -0700, Randy.Dunlap wrote:
>
> HZ may vary between different architectures. was that the autors real
> intend ?
>
> - timeout = 5 * HZ;
> + timeout = 5 * 1000;
msleep* is in milliseconds, not jiffies, so the change is correct.
thanks,
greg k-h
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace
2004-09-15 18:18 [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace Nishanth Aravamudan
` (3 preceding siblings ...)
2004-09-16 14:57 ` Greg KH
@ 2004-09-16 15:27 ` Randy.Dunlap
4 siblings, 0 replies; 6+ messages in thread
From: Randy.Dunlap @ 2004-09-16 15:27 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
On Thu, 16 Sep 2004 07:57:21 -0700 Greg KH wrote:
| On Thu, Sep 16, 2004 at 07:46:50AM -0700, Randy.Dunlap wrote:
| >
| > HZ may vary between different architectures. was that the autors real
| > intend ?
| >
| > - timeout = 5 * HZ;
| > + timeout = 5 * 1000;
|
| msleep* is in milliseconds, not jiffies, so the change is correct.
Argh, thanks. Methinks I am still msleeping.
--
~Randy
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 6+ messages in thread