[Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

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

[Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Nishanth Aravamudan]

[-- 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

Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Nishanth Aravamudan]

[-- 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

Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Randy.Dunlap]

[-- 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

Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Greg KH]

[-- 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

Re: [Kernel-janitors] [PATCH 4/5] atm/idt77252: replace

[Randy.Dunlap]

[-- 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