[PATCH] driver: ppp: Remove unnecessary bh disable in xmit path

[PATCH] driver: ppp: Remove unnecessary bh disable in xmit path

[gfree.wind]

From: Gao Feng <gfree.wind@vip.163.com>

Since the commit 55454a565836 ("ppp: avoid dealock on recursive xmit"),
the PPP xmit path is protected by wrapper functions which disable the
bh already. So it is unnecessary to disable the bh again in the real
xmit path.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 drivers/net/ppp/ppp_generic.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index f9c0e62..bbded33 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -1618,7 +1618,7 @@ static void ppp_xmit_process(struct ppp *ppp)
 		list = list->next;
 		pch = list_entry(list, struct channel, clist);
 
-		spin_lock_bh(&pch->downl);
+		spin_lock(&pch->downl);
 		if (pch->chan) {
 			if (pch->chan->ops->start_xmit(pch->chan, skb))
 				ppp->xmit_pending = NULL;
@@ -1627,7 +1627,7 @@ static void ppp_xmit_process(struct ppp *ppp)
 			kfree_skb(skb);
 			ppp->xmit_pending = NULL;
 		}
-		spin_unlock_bh(&pch->downl);
+		spin_unlock(&pch->downl);
 		return;
 	}
 
@@ -1757,7 +1757,7 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		}
 
 		/* check the channel's mtu and whether it is still attached. */
-		spin_lock_bh(&pch->downl);
+		spin_lock(&pch->downl);
 		if (pch->chan = NULL) {
 			/* can't use this channel, it's being deregistered */
 			if (pch->speed = 0)
@@ -1765,7 +1765,7 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 			else
 				totspeed -= pch->speed;
 
-			spin_unlock_bh(&pch->downl);
+			spin_unlock(&pch->downl);
 			pch->avail = 0;
 			totlen = len;
 			totfree--;
@@ -1816,7 +1816,7 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		 */
 		if (flen <= 0) {
 			pch->avail = 2;
-			spin_unlock_bh(&pch->downl);
+			spin_unlock(&pch->downl);
 			continue;
 		}
 
@@ -1861,14 +1861,14 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		len -= flen;
 		++ppp->nxseq;
 		bits = 0;
-		spin_unlock_bh(&pch->downl);
+		spin_unlock(&pch->downl);
 	}
 	ppp->nxchan = i;
 
 	return 1;
 
  noskb:
-	spin_unlock_bh(&pch->downl);
+	spin_unlock(&pch->downl);
 	if (ppp->debug & 1)
 		netdev_err(ppp->dev, "PPP: no memory (fragment)\n");
 	++ppp->dev->stats.tx_errors;
@@ -1883,7 +1883,7 @@ static void __ppp_channel_push(struct channel *pch)
 	struct sk_buff *skb;
 	struct ppp *ppp;
 
-	spin_lock_bh(&pch->downl);
+	spin_lock(&pch->downl);
 	if (pch->chan) {
 		while (!skb_queue_empty(&pch->file.xq)) {
 			skb = skb_dequeue(&pch->file.xq);
@@ -1897,14 +1897,14 @@ static void __ppp_channel_push(struct channel *pch)
 		/* channel got deregistered */
 		skb_queue_purge(&pch->file.xq);
 	}
-	spin_unlock_bh(&pch->downl);
+	spin_unlock(&pch->downl);
 	/* see if there is anything from the attached unit to be sent */
 	if (skb_queue_empty(&pch->file.xq)) {
-		read_lock_bh(&pch->upl);
+		read_lock(&pch->upl);
 		ppp = pch->ppp;
 		if (ppp)
 			__ppp_xmit_process(ppp);
-		read_unlock_bh(&pch->upl);
+		read_unlock(&pch->upl);
 	}
 }
 
-- 
1.9.1

Re: [PATCH] driver: ppp: Remove unnecessary bh disable in xmit path

[Guillaume Nault]

On Thu, Jun 01, 2017 at 09:49:18AM +0800, gfree.wind@vip.163.com wrote:
> From: Gao Feng <gfree.wind@vip.163.com>
> 
> Since the commit 55454a565836 ("ppp: avoid dealock on recursive xmit"),
> the PPP xmit path is protected by wrapper functions which disable the
> bh already. So it is unnecessary to disable the bh again in the real
> xmit path.
> 
Looks good, thanks!
But it seems you forgot to Cc. netdev@vger.kernel.org.
Please resubmit there.

You can also fix the subject to make clear this patch is for net-next.
Also, the "driver:" prefix isn't necessary.
Something like
[PATCH net-next] ppp: remove unnecessary bh disable in xmit path

Re:Re: [PATCH] driver: ppp: Remove unnecessary bh disable in xmit path

[Gao Feng]

QXQgMjAxNy0wNi0wMSAxNzowNzo1NSwgIkd1aWxsYXVtZSBOYXVsdCIgPGcubmF1bHRAYWxwaGFs
aW5rLmZyPiB3cm90ZToKPk9uIFRodSwgSnVuIDAxLCAyMDE3IGF0IDA5OjQ5OjE4QU0gKzA4MDAs
IGdmcmVlLndpbmRAdmlwLjE2My5jb20gd3JvdGU6Cj4+IEZyb206IEdhbyBGZW5nIDxnZnJlZS53
aW5kQHZpcC4xNjMuY29tPgo+PiAKPj4gU2luY2UgdGhlIGNvbW1pdCA1NTQ1NGE1NjU4MzYgKCJw
cHA6IGF2b2lkIGRlYWxvY2sgb24gcmVjdXJzaXZlIHhtaXQiKSwKPj4gdGhlIFBQUCB4bWl0IHBh
dGggaXMgcHJvdGVjdGVkIGJ5IHdyYXBwZXIgZnVuY3Rpb25zIHdoaWNoIGRpc2FibGUgdGhlCj4+
IGJoIGFscmVhZHkuIFNvIGl0IGlzIHVubmVjZXNzYXJ5IHRvIGRpc2FibGUgdGhlIGJoIGFnYWlu
IGluIHRoZSByZWFsCj4+IHhtaXQgcGF0aC4KPj4gCj5Mb29rcyBnb29kLCB0aGFua3MhCj5CdXQg
aXQgc2VlbXMgeW91IGZvcmdvdCB0byBDYy4gbmV0ZGV2QHZnZXIua2VybmVsLm9yZy4KPlBsZWFz
ZSByZXN1Ym1pdCB0aGVyZS4KCk9LLCBJIHJlc3VibWl0IGl0IGFnYWluLgoKPgo+WW91IGNhbiBh
bHNvIGZpeCB0aGUgc3ViamVjdCB0byBtYWtlIGNsZWFyIHRoaXMgcGF0Y2ggaXMgZm9yIG5ldC1u
ZXh0Lgo+QWxzbywgdGhlICJkcml2ZXI6IiBwcmVmaXggaXNuJ3QgbmVjZXNzYXJ5Lgo+U29tZXRo
aW5nIGxpa2UKPltQQVRDSCBuZXQtbmV4dF0gcHBwOiByZW1vdmUgdW5uZWNlc3NhcnkgYmggZGlz
YWJsZSBpbiB4bWl0IHBhdGgKCk5vIHByb2JsZW0sIEkgd291bGQgZm9sbG93IHlvdS4KClJlZ2Fy
ZHMKRmVuZwo