Killing ppp->dev.tbusy, TCP slow over 115200 lines?

Pavel Machek (pavel@bug.ucw.cz)
Tue, 29 Sep 1998 00:27:49 +0200


Hi!

While playing with simplex support for ppp (so I can use ppp over
IrDA), I made changes neccessary to kill ppp->dev.tbusy. Here is the
patch. I do not know if I'm not doing something very stupid, so I'd
like any comments you can give.

Anyway, please test this, and let me know how it works. If it works,
linus might like it.

Pavel

PS: I'm getting pretty poor performance: tcpspray over 115200 ppp line
does only ~6K/sec (at mru=1600), best I've seen was ~8K/sec at
mru=8000; performance dropped to 1K/sec at mru=16000. It is pretty bad
(14K/sec is absolute max, zmodem can do 11K/sec on same line). TCP
gurus, do you have any ideas?

--- clean//include/linux/if_pppvar.h Sun Jul 19 23:30:19 1998
+++ linux/include/linux/if_pppvar.h Wed Sep 23 09:59:36 1998
@@ -42,7 +42,7 @@
*/

/*
- * ==FILEVERSION 980704==
+ * ==FILEVERSION XXXXXX==
*
* NOTE TO MAINTAINERS:
* If you modify this file at all, please set the above date.
@@ -114,6 +114,8 @@

/* Statistic information */
struct pppstat stats; /* statistic information */
+
+ u32 xmit_lock;

/* PPP compression protocol information */
struct compressor *sc_xcomp; /* transmit compressor */
--- clean//drivers/net/ppp.c Wed Aug 26 13:09:37 1998
+++ linux/drivers/net/ppp.c Tue Sep 29 00:17:19 1998
@@ -4,7 +4,7 @@
* Al Longyear <longyear@netcom.com>
* Extensively rewritten by Paul Mackerras <paulus@cs.anu.edu.au>
*
- * ==FILEVERSION 980704==
+ * ==FILEVERSION XXXXXX==
*
* NOTE TO MAINTAINERS:
* If you modify this file at all, please set the number above to the
@@ -115,6 +118,7 @@
static void ppp_output_wakeup(struct ppp *ppp);
static void ppp_send_ctrl(struct ppp *ppp, struct sk_buff *skb);
static void ppp_send_frame(struct ppp *ppp, struct sk_buff *skb);
+static inline void ppp_send_frames(struct ppp *ppp);
static struct sk_buff *ppp_vj_compress(struct ppp *ppp, struct sk_buff *skb);

static struct ppp *ppp_find (int pid_value);
@@ -2112,8 +2298,8 @@

/*
* Compress and send an frame to the peer.
- * Should be called with dev->tbusy == 1, having been set by the caller.
- * That is, we use dev->tbusy as a lock to prevent reentry of this
+ * Should be called with xmit_lock == 1, having been set by the caller.
+ * That is, we use xmit_lock as a lock to prevent reentry of this
* procedure.
*/
static void
@@ -2185,7 +2371,7 @@
if (new_skb == NULL) {
printk(KERN_ERR "ppp_send_frame: no memory\n");
kfree_skb(skb);
- ppp->dev.tbusy = 0;
+ ppp->xmit_lock = 0;
return;
}

@@ -2214,7 +2400,7 @@
ret = ppp_async_send(ppp, skb);
if (ret > 0) {
/* we can release the lock */
- ppp->dev.tbusy = 0;
+ ppp->xmit_lock = 0;
} else if (ret < 0) {
- /* this can't happen, since the caller got the tbusy lock */
+ /* this can't happen, since the caller got the xmit_lock */
printk(KERN_ERR "ppp: ppp_async_send didn't accept pkt\n");
@@ -2275,9 +2465,9 @@
{
struct sk_buff *skb;

+ while (!test_and_set_bit(0, &ppp->xmit_lock)) {
skb = skb_dequeue(&ppp->xmt_q);
if (skb == NULL) {
- ppp->dev.tbusy = 0;
+ ppp->xmit_lock = 0;
- mark_bh(NET_BH);
break;
}
@@ -2295,11 +2489,11 @@
{
CHECK_PPP_VOID();

- if (!ppp->dev.tbusy) {
- printk(KERN_ERR "ppp_output_wakeup called but tbusy==0\n");
+ if (!ppp->xmit_lock) {
+ printk(KERN_ERR "ppp_output_wakeup called but xmit_lock==0\n");
return;
}
- ppp->dev.tbusy = 0;
+ ppp->xmit_lock = 0;
ppp_send_frames(ppp);
}

@@ -2421,14 +2615,6 @@
}

/*
- * The dev->tbusy field acts as a lock to allow only
- * one packet to be processed at a time. If we can't
- * get the lock, try again later.
- */
- if (test_and_set_bit(0, &dev->tbusy))
- return 1;
-
- /*
* Put the 4-byte PPP header on the packet.
* If there isn't room for it, we have to copy the packet.
*/
@@ -2440,7 +2626,6 @@
printk(KERN_ERR "%s: skb hdr alloc failed\n",
ppp->name);
dev_kfree_skb(skb);
- dev->tbusy = 0;
return 0;
}
skb_reserve(new_skb, PPP_HDRLEN);
@@ -2455,7 +2640,8 @@
hdr[2] = proto >> 8;
hdr[3] = proto;

- ppp_send_frame(ppp, skb);
+ skb_queue_tail(&ppp->xmt_q, skb);
+ ppp_send_frames(ppp);
return 0;
}

@@ -2597,8 +2789,9 @@
skb_queue_head_init(&ppp->xmt_q);
skb_queue_head_init(&ppp->rcv_q);

- ppp->last_xmit = jiffies;
- ppp->last_recv = jiffies;
+ ppp->last_xmit = ppp->last_recv = jiffies;
+
+ ppp->xmit_lock = 0;

/* clear statistics */
memset(&ppp->stats, 0, sizeof (struct pppstat));

-- 
I'm really pavel@atrey.karlin.mff.cuni.cz. 	   Pavel
Look at http://atrey.karlin.mff.cuni.cz/~pavel/ ;-).

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