atm/idt77252: replace

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.

@@ -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;
 		}

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

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.

@@ -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;
 		}

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

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.

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[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 <[email protected]>
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.

@@ -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;
 		}
 

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

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

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors