From patchwork Mon Mar 26 10:03:42 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 148682 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 302FFB6FA4 for ; Mon, 26 Mar 2012 21:04:20 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756334Ab2CZKD7 (ORCPT ); Mon, 26 Mar 2012 06:03:59 -0400 Received: from merlin.infradead.org ([205.233.59.134]:49660 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756743Ab2CZKDx (ORCPT ); Mon, 26 Mar 2012 06:03:53 -0400 Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1SC6mB-0006Mw-Tp; Mon, 26 Mar 2012 10:03:47 +0000 Received: from dyn-247.woodhou.se ([90.155.92.247]) by canuck.infradead.org with esmtpsa (Exim 4.76 #1 (Red Hat Linux)) id 1SC6mB-0007Ki-9s; Mon, 26 Mar 2012 10:03:47 +0000 Message-ID: <1332756222.2379.66.camel@shinybook.infradead.org> Subject: [PATCH] ppp: Don't stop and restart queue on every TX packet From: David Woodhouse To: David Miller , Paul Mackerras Cc: netdev@vger.kernel.org Date: Mon, 26 Mar 2012 11:03:42 +0100 In-Reply-To: <20120325.173635.1909319488008466320.davem@davemloft.net> References: <1332450218.32446.79.camel@shinybook.infradead.org> <20120322.230331.1623101647193498167.davem@davemloft.net> <1332672230.32446.160.camel@shinybook.infradead.org> <20120325.173635.1909319488008466320.davem@davemloft.net> X-Mailer: Evolution 3.2.3 (3.2.3-2.fc16) Mime-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by canuck.infradead.org See http://www.infradead.org/rpr.html Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org For every transmitted packet, ppp_start_xmit() will stop the netdev queue and then, if appropriate, restart it. This causes the TX softirq to run, entirely gratuitously. This is "only" a waste of CPU time in the normal case, but it's actively harmful when the PPP device is a TEQL slave — the wakeup will cause the offending device to receive the next TX packet from the TEQL queue, when it *should* have gone to the next slave in the list. We end up seeing large bursts of packets on just *one* slave device, rather than using the full available bandwidth over all slaves. This patch fixes the problem by *not* unconditionally stopping the queue in ppp_start_xmit(). It adds a return value from ppp_xmit_process() which indicates whether the queue should be stopped or not. It *doesn't* remove the call to netif_wake_queue() from ppp_xmit_process(), because other code paths (especially from ppp_output_wakeup()) need it there and it's messy to push it out to the other callers to do it based on the return value. So we leave it in place — it's a no-op in the case where the queue wasn't stopped, so it's harmless in the TX path. Signed-off-by: David Woodhouse --- drivers/net/ppp/ppp_generic.c~ 2012-01-26 00:39:32.000000000 +0000 +++ drivers/net/ppp/ppp_generic.c 2012-03-26 10:32:31.286744147 +0100 @@ -235,7 +235,7 @@ struct ppp_net { /* Prototypes. */ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf, struct file *file, unsigned int cmd, unsigned long arg); -static void ppp_xmit_process(struct ppp *ppp); +static int ppp_xmit_process(struct ppp *ppp); static void ppp_send_frame(struct ppp *ppp, struct sk_buff *skb); static void ppp_push(struct ppp *ppp); static void ppp_channel_push(struct channel *pch); @@ -968,9 +968,9 @@ ppp_start_xmit(struct sk_buff *skb, stru proto = npindex_to_proto[npi]; put_unaligned_be16(proto, pp); - netif_stop_queue(dev); skb_queue_tail(&ppp->file.xq, skb); - ppp_xmit_process(ppp); + if (!ppp_xmit_process(ppp)) + netif_stop_queue(dev); return NETDEV_TX_OK; outf: @@ -1048,10 +1048,11 @@ static void ppp_setup(struct net_device * Called to do any work queued up on the transmit side * that can now be done. */ -static void +static int ppp_xmit_process(struct ppp *ppp) { struct sk_buff *skb; + int ret = 0; ppp_xmit_lock(ppp); if (!ppp->closing) { @@ -1061,10 +1062,13 @@ ppp_xmit_process(struct ppp *ppp) ppp_send_frame(ppp, skb); /* If there's no work left to do, tell the core net code that we can accept some more. */ - if (!ppp->xmit_pending && !skb_peek(&ppp->file.xq)) + if (!ppp->xmit_pending && !skb_peek(&ppp->file.xq)) { netif_wake_queue(ppp->dev); + ret = 1; + } } ppp_xmit_unlock(ppp); + return ret; } static inline struct sk_buff *