Messages in this thread Patch in this message | | | Date | Tue, 16 Feb 1999 19:39:35 +0100 (CET) | From | Andrea Arcangeli <> | Subject | Re: [patch] shaper fix for timer SMP races [was Re: 2.2.2-pre2 hangs] |
| |
On Tue, 16 Feb 1999, Andrea Arcangeli wrote:
>I developed a new patch that I sent to Alex now for testing. The new patch
Alex didn't replyed me yet but since I had some time this evening I recompiled the kernel with shaper support and I tried the shaper + my patch myself.
With my new patch applyed the shaper works fine here (never tried it without the patch though). The performance penality is zero, I get the _same_ throughput as without putting the shaper between TCP/IP and the raw eth0 (ftp runs 800kbyte/sec as usual).
Here my patch I sent to Alex this afternoon:
Index: drivers/net/shaper.c =================================================================== RCS file: /var/cvs/linux/drivers/net/shaper.c,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 shaper.c --- shaper.c 1999/01/18 01:28:09 1.1.1.1 +++ linux/drivers/net/shaper.c 1999/02/16 13:25:36 @@ -53,6 +53,13 @@ * This will be fixed in BETA4 */ +/* + * bh_atomic() SMP races fixes and rewritten the locking code to be SMP safe + * and irq-mask friendly. NOTE: we can't use start_bh_atomic() in kick_shaper() + * because it's going to be recalled from an irq handler, and synchronize_bh() + * is a nono if called from irq context. + * 1999 Andrea Arcangeli + */ #include <linux/module.h> #include <linux/kernel.h> @@ -83,21 +90,17 @@ static int shaper_lock(struct shaper *sh) { - unsigned long flags; - save_flags(flags); - cli(); /* - * Lock in an interrupt may fail + * Lock in an interrupt must fail */ - if(sh->locked && in_interrupt()) + while (test_and_set_bit(0, &sh->locked)) { - restore_flags(flags); - return 0; + if (!in_interrupt()) + sleep_on(&sh->wait_queue); + else + return 0; + } - while(sh->locked) - sleep_on(&sh->wait_queue); - sh->locked=1; - restore_flags(flags); return 1; } @@ -105,7 +108,7 @@ static void shaper_unlock(struct shaper *sh) { - sh->locked=0; + clear_bit(0, &sh->locked); wake_up(&sh->wait_queue); shaper_kick(sh); } @@ -240,7 +243,6 @@ dev_kfree_skb(ptr); } shaper_unlock(shaper); - shaper_kick(shaper); return 0; } @@ -285,24 +287,16 @@ static void shaper_kick(struct shaper *shaper) { struct sk_buff *skb; - unsigned long flags; - save_flags(flags); - cli(); - - del_timer(&shaper->timer); - /* * Shaper unlock will kick */ - if(shaper->locked) - { + if (test_and_set_bit(0, &shaper->locked)) + { if(sh_debug) printk("Shaper locked.\n"); - shaper->timer.expires=jiffies+1; - add_timer(&shaper->timer); - restore_flags(flags); + mod_timer(&shaper->timer, jiffies); return; } @@ -320,7 +314,7 @@ if(sh_debug) printk("Clock = %d, jiffies = %ld\n", skb->shapeclock, jiffies); - if(skb->shapeclock - jiffies <= SHAPER_BURST) + if(time_before_eq(skb->shapeclock - jiffies, SHAPER_BURST)) { /* * Pull the frame and get interrupts back on. @@ -329,8 +323,6 @@ skb_unlink(skb); if (shaper->recovery < skb->shapeclock + skb->shapelen) shaper->recovery = skb->shapeclock + skb->shapelen; - restore_flags(flags); - /* * Pass on to the physical target device via * our low level packet thrower. @@ -338,7 +330,6 @@ skb->shapepend=0; shaper_queue_xmit(shaper, skb); /* Fire */ - cli(); } else break; @@ -349,17 +340,9 @@ */ if(skb!=NULL) - { - del_timer(&shaper->timer); - shaper->timer.expires=skb->shapeclock; - add_timer(&shaper->timer); - } - - /* - * Interrupts on, mission complete - */ - - restore_flags(flags); + mod_timer(&shaper->timer, skb->shapeclock); + + clear_bit(0, &shaper->locked); } @@ -370,8 +353,14 @@ static void shaper_flush(struct shaper *shaper) { struct sk_buff *skb; + if(!shaper_lock(shaper)) + { + printk(KERN_ERR "shaper: shaper_flush() called by an irq!\n"); + return; + } while((skb=skb_dequeue(&shaper->sendq))!=NULL) dev_kfree_skb(skb); + shaper_unlock(shaper); } /* @@ -405,7 +394,9 @@ { struct shaper *shaper=dev->priv; shaper_flush(shaper); + start_bh_atomic(); del_timer(&shaper->timer); + end_bh_atomic(); MOD_DEC_USE_COUNT; return 0; } Index: include/linux/if_shaper.h =================================================================== RCS file: /var/cvs/linux/include/linux/if_shaper.h,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 if_shaper.h --- if_shaper.h 1999/01/18 01:27:12 1.1.1.1 +++ if_shaper.h 1999/02/16 12:20:37 @@ -23,7 +23,7 @@ __u32 shapeclock; __u32 recovery; /* Time we can next clock a packet out on an empty queue */ - char locked; + unsigned long locked; struct device *dev; int (*hard_start_xmit) (struct sk_buff *skb, struct device *dev);
Andrea Arcangeli
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |