Skip to contentSkip to conversation

net/ibmtr: replace schedule_timeout() with msleep()/msleep_interruptible()

PATCH In review [email protected] 3 participants 4 messages Oct 30, 2004 – Nov 1, 2004

Subsystems (3)
Networking Network drivers Kernel janitors
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 15/18 net/ibmtr: replace schedule_timeout() with msleep()/msleep_interruptible()

Any comments would be appreciated.

Description: Use msleep() / msleep_interruptible() [as appropriate] instead of schedule_timeout() to guarantee the task delays as expected.

Signed-off-by Nishanth Aravamudan <[email protected]>
Signed-off-by Maximilian Attems <[email protected]>

---

 linux-2.6.10-rc1-max/drivers/net/tokenring/ibmtr.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
drivers/net/tokenring/ibmtr.c renamed from drivers/net/tokenring/ibmtr.c~msleep+msleep_interruptible-drivers_net_tokenring_ibmtr +5 -6
diff -puN drivers/net/tokenring/ibmtr.c~msleep+msleep_interruptible-drivers_net_tokenring_ibmtr drivers/net/tokenring/ibmtr.c
@@ -108,6 +108,7 @@ in the event that chatty debug messages 
 #define IBMTR_DEBUG_MESSAGES 0
 
 #include <linux/module.h>
+#include <linux/delay.h>
 
 #ifdef PCMCIA		/* required for ibmtr_cs.c to build */
 #undef MODULE		/* yes, really */

@@ -317,7 +318,7 @@ static void ibmtr_cleanup_card(struct ne
 	if (dev->base_addr) {
 		outb(0,dev->base_addr+ADAPTRESET);
 		
-		schedule_timeout(TR_RST_TIME); /* wait 50ms */
+		msleep(jiffies_to_msecs(TR_RST_TIME)); /* wait 50ms */
 
 		outb(0,dev->base_addr+ADAPTRESETREL);
 	}

@@ -858,8 +859,7 @@ static int tok_init_card(struct net_devi
 	writeb(~INT_ENABLE, ti->mmio + ACA_OFFSET + ACA_RESET + ISRP_EVEN);
 	outb(0, PIOaddr + ADAPTRESET);
 
-	current->state=TASK_UNINTERRUPTIBLE;
-	schedule_timeout(TR_RST_TIME); /* wait 50ms */
+	msleep(jiffies_to_msecs(TR_RST_TIME));		/* wait 50ms */
 
 	outb(0, PIOaddr + ADAPTRESETREL);
 #ifdef ENABLE_PAGING

@@ -912,9 +912,8 @@ static int tok_open(struct net_device *d
 			DPRINTK("Adapter is up and running\n");
 			return 0;
 		}
-		current->state=TASK_INTERRUPTIBLE;
-		i=schedule_timeout(TR_RETRY_INTERVAL); /* wait 30 seconds */
-		if(i!=0) break; /*prob. a signal, like the i>24*HZ case above */
+		if(msleep_interruptible(jiffies_to_msecs(TR_RETRY_INTERVAL)))
+			break; /*prob. a signal, like the i>24*HZ case above */
 	}
 	outb(0, dev->base_addr + ADAPTRESET);/* kill pending interrupts*/
 	DPRINTK("TERMINATED via signal\n");	/*BMS useful */

_

#1 lore raw
Quoted 45 lines · [email protected] wrote:

Any comments would be appreciated.

Description: Use msleep() / msleep_interruptible() [as appropriate] instead of schedule_timeout() to guarantee the task delays as expected.

Signed-off-by Nishanth Aravamudan <[email protected]>
Signed-off-by Maximilian Attems <[email protected]>

---

 linux-2.6.10-rc1-max/drivers/net/tokenring/ibmtr.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff -puN drivers/net/tokenring/ibmtr.c~msleep+msleep_interruptible-drivers_net_tokenring_ibmtr drivers/net/tokenring/ibmtr.c

drivers/net/tokenring/ibmtr.c renamed from drivers/net/tokenring/ibmtr.c~msleep+msleep_interruptible-drivers_net_tokenring_ibmtr +3 -3
@@ -108,6 +108,7 @@ in the event that chatty debug messages 
 #define IBMTR_DEBUG_MESSAGES 0
 
 #include <linux/module.h>
+#include <linux/delay.h>
 
 #ifdef PCMCIA		/* required for ibmtr_cs.c to build */
 #undef MODULE		/* yes, really */

@@ -317,7 +318,7 @@ static void ibmtr_cleanup_card(struct ne
 	if (dev->base_addr) {
 		outb(0,dev->base_addr+ADAPTRESET);
 
-		schedule_timeout(TR_RST_TIME); /* wait 50ms */
+		msleep(jiffies_to_msecs(TR_RST_TIME)); /* wait 50ms */
 
 		outb(0,dev->base_addr+ADAPTRESETREL);
 	}

@@ -858,8 +859,7 @@ static int tok_init_card(struct net_devi
 	writeb(~INT_ENABLE, ti->mmio + ACA_OFFSET + ACA_RESET + ISRP_EVEN);
 	outb(0, PIOaddr + ADAPTRESET);
 
-	current->state=TASK_UNINTERRUPTIBLE;
-	schedule_timeout(TR_RST_TIME); /* wait 50ms */
+	msleep(jiffies_to_msecs(TR_RST_TIME));		/* wait 50ms */
 
 	outb(0, PIOaddr + ADAPTRESETREL);
 #ifdef ENABLE_PAGING

@@ -912,9 +912,8 @@ static int tok_open(struct net_device *d
 			DPRINTK("Adapter is up and running\n");

drivers/net/tokenring/ibmtr.c static int tok_open(struct net_device *d · line 919

Quoted 10 lines
drivers/net/tokenring/ibmtr.c renamed from drivers/net/tokenring/ibmtr.c~msleep+msleep_interruptible-drivers_net_tokenring_ibmtr +2 -3
 			return 0;
 		}
-		current->state=TASK_INTERRUPTIBLE;
-		i=schedule_timeout(TR_RETRY_INTERVAL); /* wait 30 seconds */
-		if(i!=0) break; /*prob. a signal, like the i>24*HZ case above */
+		if(msleep_interruptible(jiffies_to_msecs(TR_RETRY_INTERVAL)))
+			break; /*prob. a signal, like the i>24*HZ case above */
 	}
 	outb(0, dev->base_addr + ADAPTRESET);/* kill pending interrupts*/
 	DPRINTK("TERMINATED via signal\n");	/*BMS useful */

It makes more sense to convert the constants to msecs in the source code, instead of converting them at runtime.

Jeff

#2 lore raw

PATCH 1/1 net/ibmtr: replace schedule_timeout() with msleep()/msleep_interruptible()

Quoted 16 lines · Jeff Garzik
Quoted 15 lines · [email protected] wrote:

Any comments would be appreciated.

Description: Use msleep() / msleep_interruptible() [as appropriate] instead of schedule_timeout() to guarantee the task delays as expected.

Signed-off-by Nishanth Aravamudan <[email protected]>
Signed-off-by Maximilian Attems <[email protected]>

---

linux-2.6.10-rc1-max/drivers/net/tokenring/ibmtr.c | 11 +++++------

1 files changed, 5 insertions(+), 6 deletions(-)
(quoted diff) +1 -1
-		schedule_timeout(TR_RST_TIME); /* wait 50ms */
+		msleep(jiffies_to_msecs(TR_RST_TIME)); /* wait 50ms */

<snip>

Quoted 3 lines
Quoted 3 lines
(quoted diff) +1 -2
-	current->state=TASK_UNINTERRUPTIBLE;
-	schedule_timeout(TR_RST_TIME); /* wait 50ms */
+	msleep(jiffies_to_msecs(TR_RST_TIME));		/* wait 50ms */

<snip>

Quoted 6 lines
Quoted 6 lines
(quoted diff) +0 -3
-		current->state=TASK_INTERRUPTIBLE;
-		i=schedule_timeout(TR_RETRY_INTERVAL); /* wait 30 seconds */
-		if(i!=0) break; /*prob. a signal, like the i>24*HZ case

above */

(quoted diff) +2 -0
+		if(msleep_interruptible(jiffies_to_msecs(TR_RETRY_INTERVAL)))
+			break; /*prob. a signal, like the i>24*HZ case above

<snip>

Quoted 2 lines

It makes more sense to convert the constants to msecs in the source code, instead of converting them at runtime.

Here is the patch to ibmtr.c which completes these changes...

Description: Uses msleep()/msleep_interruptible() [as appropriate] to guarantee the task delays as expected. This patch depends on the corresponding addition of the millisecond-unit constants to ibmtr.h.

Signed-off-by Nishanth Aravamudan <[email protected]>
drivers/net/tokenring/ibmtr.c +5 -6
@@ -130,6 +130,7 @@ in the event that chatty debug messages 
 #include <linux/ip.h>
 #include <linux/trdevice.h>
 #include <linux/ibmtr.h>
+#include <linux/delay.h>
 
 #include <net/checksum.h>
 

@@ -317,7 +318,7 @@ static void ibmtr_cleanup_card(struct ne
 	if (dev->base_addr) {
 		outb(0,dev->base_addr+ADAPTRESET);
 		
-		schedule_timeout(TR_RST_TIME); /* wait 50ms */
+		msleep(TR_RST_TIME_MS);
 
 		outb(0,dev->base_addr+ADAPTRESETREL);
 	}

@@ -858,8 +859,7 @@ static int tok_init_card(struct net_devi
 	writeb(~INT_ENABLE, ti->mmio + ACA_OFFSET + ACA_RESET + ISRP_EVEN);
 	outb(0, PIOaddr + ADAPTRESET);
 
-	current->state=TASK_UNINTERRUPTIBLE;
-	schedule_timeout(TR_RST_TIME); /* wait 50ms */
+	msleep(TR_RST_TIME_MS);
 
 	outb(0, PIOaddr + ADAPTRESETREL);
 #ifdef ENABLE_PAGING

@@ -912,9 +912,8 @@ static int tok_open(struct net_device *d
 			DPRINTK("Adapter is up and running\n");
 			return 0;
 		}
-		current->state=TASK_INTERRUPTIBLE;
-		i=schedule_timeout(TR_RETRY_INTERVAL); /* wait 30 seconds */
-		if(i!=0) break; /*prob. a signal, like the i>24*HZ case above */
+		if(msleep_interruptible(TR_RETRY_INTERVAL_MS)) break;
+				/*prob. a signal, like the i>24*HZ case above */
 	}
 	outb(0, dev->base_addr + ADAPTRESET);/* kill pending interrupts*/
 	DPRINTK("TERMINATED via signal\n");	/*BMS useful */

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

#3 lore raw

PATCH 1/1 linux/ibmtr.h: add constants for appropriate delays

In reply to Jeff Garzik
Quoted 13 lines · Jeff Garzik
Quoted 12 lines · [email protected] wrote:

Any comments would be appreciated.

Description: Use msleep() / msleep_interruptible() [as appropriate] instead of schedule_timeout() to guarantee the task delays as expected.

Signed-off-by Nishanth Aravamudan <[email protected]>
Signed-off-by Maximilian Attems <[email protected]>

---

linux-2.6.10-rc1-max/drivers/net/tokenring/ibmtr.c | 11 +++++------

1 files changed, 5 insertions(+), 6 deletions(-)

<snip>

Quoted 2 lines
Quoted 2 lines
(quoted diff) +1 -1
-		schedule_timeout(TR_RST_TIME); /* wait 50ms */
+		msleep(jiffies_to_msecs(TR_RST_TIME)); /* wait 50ms */

<snip>

Quoted 3 lines
Quoted 3 lines
(quoted diff) +1 -2
-	current->state=TASK_UNINTERRUPTIBLE;
-	schedule_timeout(TR_RST_TIME); /* wait 50ms */
+	msleep(jiffies_to_msecs(TR_RST_TIME));		/* wait 50ms */

<snip>

Quoted 6 lines
Quoted 6 lines
(quoted diff) +0 -3
-		current->state=TASK_INTERRUPTIBLE;
-		i=schedule_timeout(TR_RETRY_INTERVAL); /* wait 30 seconds */
-		if(i!=0) break; /*prob. a signal, like the i>24*HZ case

above */

(quoted diff) +2 -0
+		if(msleep_interruptible(jiffies_to_msecs(TR_RETRY_INTERVAL)))
+			break; /*prob. a signal, like the i>24*HZ case above

<snip>

Quoted 2 lines

It makes more sense to convert the constants to msecs in the source code, instead of converting them at runtime.

This required changing both the .h and .c files, the first of which is attached below.

Description: Adds constant definitions for timeout variables in terms of milliseconds to avoid run-time constant conversion.

Signed-off-by Nishanth Aravamudan <[email protected]>
include/linux/ibmtr.h +4 -2
@@ -6,8 +6,10 @@
 
 /* ported to the Alpha architecture 02/20/96 (just used the HZ macro) */
 
-#define TR_RETRY_INTERVAL	(30*HZ)	/* 500 on PC = 5 s */
-#define TR_RST_TIME		(HZ/20) /* 5 on PC = 50 ms */
+#define TR_RETRY_INTERVAL	(30*HZ)	/* 500 on PC = 5 s (in jiffies) */
+#define TR_RETRY_INTERVAL	(30000) /* 500 on PC = 5 s */
+#define TR_RST_TIME		(HZ/20) /* 5 on PC = 50 ms (in jiffies) */
+#define TR_RST_TIME_MS		(50)    /* 5 on PC = 50 ms */
 #define TR_BUSY_INTERVAL	(HZ/5)	/* 5 on PC = 200 ms */
 #define TR_SPIN_INTERVAL	(3*HZ)	/* 3 seconds before init timeout */

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