|
|
@@ -0,0 +1,334 @@
|
|
|
+From b8844aab519a154808dbce15a132f3e8f1c34af6 Mon Sep 17 00:00:00 2001
|
|
|
+From: Qingfang Deng <[email protected]>
|
|
|
+Date: Fri, 22 Aug 2025 09:25:47 +0800
|
|
|
+Subject: [PATCH] ppp: remove rwlock usage
|
|
|
+
|
|
|
+In struct channel, the upl lock is implemented using rwlock_t,
|
|
|
+protecting access to pch->ppp and pch->bridge.
|
|
|
+
|
|
|
+As previously discussed on the list, using rwlock in the network fast
|
|
|
+path is not recommended.
|
|
|
+This patch replaces the rwlock with a spinlock for writers, and uses RCU
|
|
|
+for readers.
|
|
|
+
|
|
|
+- pch->ppp and pch->bridge are now declared as __rcu pointers.
|
|
|
+- Readers use rcu_dereference_bh() under rcu_read_lock_bh().
|
|
|
+- Writers use spin_lock() to update.
|
|
|
+
|
|
|
+Signed-off-by: Qingfang Deng <[email protected]>
|
|
|
+Link: https://patch.msgid.link/[email protected]
|
|
|
+Signed-off-by: Jakub Kicinski <[email protected]>
|
|
|
+---
|
|
|
+ drivers/net/ppp/ppp_generic.c | 120 ++++++++++++++++++----------------
|
|
|
+ 1 file changed, 63 insertions(+), 57 deletions(-)
|
|
|
+
|
|
|
+--- a/drivers/net/ppp/ppp_generic.c
|
|
|
++++ b/drivers/net/ppp/ppp_generic.c
|
|
|
+@@ -173,11 +173,11 @@ struct channel {
|
|
|
+ struct ppp_channel *chan; /* public channel data structure */
|
|
|
+ struct rw_semaphore chan_sem; /* protects `chan' during chan ioctl */
|
|
|
+ spinlock_t downl; /* protects `chan', file.xq dequeue */
|
|
|
+- struct ppp *ppp; /* ppp unit we're connected to */
|
|
|
++ struct ppp __rcu *ppp; /* ppp unit we're connected to */
|
|
|
+ struct net *chan_net; /* the net channel belongs to */
|
|
|
+ netns_tracker ns_tracker;
|
|
|
+ struct list_head clist; /* link in list of channels per unit */
|
|
|
+- rwlock_t upl; /* protects `ppp' and 'bridge' */
|
|
|
++ spinlock_t upl; /* protects `ppp' and 'bridge' */
|
|
|
+ struct channel __rcu *bridge; /* "bridged" ppp channel */
|
|
|
+ #ifdef CONFIG_PPP_MULTILINK
|
|
|
+ u8 avail; /* flag used in multilink stuff */
|
|
|
+@@ -639,34 +639,34 @@ static struct bpf_prog *compat_ppp_get_f
|
|
|
+ */
|
|
|
+ static int ppp_bridge_channels(struct channel *pch, struct channel *pchb)
|
|
|
+ {
|
|
|
+- write_lock_bh(&pch->upl);
|
|
|
+- if (pch->ppp ||
|
|
|
++ spin_lock(&pch->upl);
|
|
|
++ if (rcu_dereference_protected(pch->ppp, lockdep_is_held(&pch->upl)) ||
|
|
|
+ rcu_dereference_protected(pch->bridge, lockdep_is_held(&pch->upl))) {
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+ return -EALREADY;
|
|
|
+ }
|
|
|
+ refcount_inc(&pchb->file.refcnt);
|
|
|
+ rcu_assign_pointer(pch->bridge, pchb);
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+
|
|
|
+- write_lock_bh(&pchb->upl);
|
|
|
+- if (pchb->ppp ||
|
|
|
++ spin_lock(&pchb->upl);
|
|
|
++ if (rcu_dereference_protected(pchb->ppp, lockdep_is_held(&pchb->upl)) ||
|
|
|
+ rcu_dereference_protected(pchb->bridge, lockdep_is_held(&pchb->upl))) {
|
|
|
+- write_unlock_bh(&pchb->upl);
|
|
|
++ spin_unlock(&pchb->upl);
|
|
|
+ goto err_unset;
|
|
|
+ }
|
|
|
+ refcount_inc(&pch->file.refcnt);
|
|
|
+ rcu_assign_pointer(pchb->bridge, pch);
|
|
|
+- write_unlock_bh(&pchb->upl);
|
|
|
++ spin_unlock(&pchb->upl);
|
|
|
+
|
|
|
+ return 0;
|
|
|
+
|
|
|
+ err_unset:
|
|
|
+- write_lock_bh(&pch->upl);
|
|
|
++ spin_lock(&pch->upl);
|
|
|
+ /* Re-read pch->bridge with upl held in case it was modified concurrently */
|
|
|
+ pchb = rcu_dereference_protected(pch->bridge, lockdep_is_held(&pch->upl));
|
|
|
+ RCU_INIT_POINTER(pch->bridge, NULL);
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+ synchronize_rcu();
|
|
|
+
|
|
|
+ if (pchb)
|
|
|
+@@ -680,25 +680,25 @@ static int ppp_unbridge_channels(struct
|
|
|
+ {
|
|
|
+ struct channel *pchb, *pchbb;
|
|
|
+
|
|
|
+- write_lock_bh(&pch->upl);
|
|
|
++ spin_lock(&pch->upl);
|
|
|
+ pchb = rcu_dereference_protected(pch->bridge, lockdep_is_held(&pch->upl));
|
|
|
+ if (!pchb) {
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+ return -EINVAL;
|
|
|
+ }
|
|
|
+ RCU_INIT_POINTER(pch->bridge, NULL);
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+
|
|
|
+ /* Only modify pchb if phcb->bridge points back to pch.
|
|
|
+ * If not, it implies that there has been a race unbridging (and possibly
|
|
|
+ * even rebridging) pchb. We should leave pchb alone to avoid either a
|
|
|
+ * refcount underflow, or breaking another established bridge instance.
|
|
|
+ */
|
|
|
+- write_lock_bh(&pchb->upl);
|
|
|
++ spin_lock(&pchb->upl);
|
|
|
+ pchbb = rcu_dereference_protected(pchb->bridge, lockdep_is_held(&pchb->upl));
|
|
|
+ if (pchbb == pch)
|
|
|
+ RCU_INIT_POINTER(pchb->bridge, NULL);
|
|
|
+- write_unlock_bh(&pchb->upl);
|
|
|
++ spin_unlock(&pchb->upl);
|
|
|
+
|
|
|
+ synchronize_rcu();
|
|
|
+
|
|
|
+@@ -2144,10 +2144,9 @@ static int ppp_mp_explode(struct ppp *pp
|
|
|
+ #endif /* CONFIG_PPP_MULTILINK */
|
|
|
+
|
|
|
+ /* Try to send data out on a channel */
|
|
|
+-static void __ppp_channel_push(struct channel *pch)
|
|
|
++static void __ppp_channel_push(struct channel *pch, struct ppp *ppp)
|
|
|
+ {
|
|
|
+ struct sk_buff *skb;
|
|
|
+- struct ppp *ppp;
|
|
|
+
|
|
|
+ spin_lock(&pch->downl);
|
|
|
+ if (pch->chan) {
|
|
|
+@@ -2166,7 +2165,6 @@ static void __ppp_channel_push(struct ch
|
|
|
+ spin_unlock(&pch->downl);
|
|
|
+ /* see if there is anything from the attached unit to be sent */
|
|
|
+ if (skb_queue_empty(&pch->file.xq)) {
|
|
|
+- ppp = pch->ppp;
|
|
|
+ if (ppp)
|
|
|
+ __ppp_xmit_process(ppp, NULL);
|
|
|
+ }
|
|
|
+@@ -2174,15 +2172,18 @@ static void __ppp_channel_push(struct ch
|
|
|
+
|
|
|
+ static void ppp_channel_push(struct channel *pch)
|
|
|
+ {
|
|
|
+- read_lock_bh(&pch->upl);
|
|
|
+- if (pch->ppp) {
|
|
|
+- (*this_cpu_ptr(pch->ppp->xmit_recursion))++;
|
|
|
+- __ppp_channel_push(pch);
|
|
|
+- (*this_cpu_ptr(pch->ppp->xmit_recursion))--;
|
|
|
++ struct ppp *ppp;
|
|
|
++
|
|
|
++ rcu_read_lock_bh();
|
|
|
++ ppp = rcu_dereference_bh(pch->ppp);
|
|
|
++ if (ppp) {
|
|
|
++ (*this_cpu_ptr(ppp->xmit_recursion))++;
|
|
|
++ __ppp_channel_push(pch, ppp);
|
|
|
++ (*this_cpu_ptr(ppp->xmit_recursion))--;
|
|
|
+ } else {
|
|
|
+- __ppp_channel_push(pch);
|
|
|
++ __ppp_channel_push(pch, NULL);
|
|
|
+ }
|
|
|
+- read_unlock_bh(&pch->upl);
|
|
|
++ rcu_read_unlock_bh();
|
|
|
+ }
|
|
|
+
|
|
|
+ /*
|
|
|
+@@ -2284,6 +2285,7 @@ void
|
|
|
+ ppp_input(struct ppp_channel *chan, struct sk_buff *skb)
|
|
|
+ {
|
|
|
+ struct channel *pch = chan->ppp;
|
|
|
++ struct ppp *ppp;
|
|
|
+ int proto;
|
|
|
+
|
|
|
+ if (!pch) {
|
|
|
+@@ -2295,18 +2297,19 @@ ppp_input(struct ppp_channel *chan, stru
|
|
|
+ if (ppp_channel_bridge_input(pch, skb))
|
|
|
+ return;
|
|
|
+
|
|
|
+- read_lock_bh(&pch->upl);
|
|
|
++ rcu_read_lock_bh();
|
|
|
++ ppp = rcu_dereference_bh(pch->ppp);
|
|
|
+ if (!ppp_decompress_proto(skb)) {
|
|
|
+ kfree_skb(skb);
|
|
|
+- if (pch->ppp) {
|
|
|
+- ++pch->ppp->dev->stats.rx_length_errors;
|
|
|
+- ppp_receive_error(pch->ppp);
|
|
|
++ if (ppp) {
|
|
|
++ ++ppp->dev->stats.rx_length_errors;
|
|
|
++ ppp_receive_error(ppp);
|
|
|
+ }
|
|
|
+ goto done;
|
|
|
+ }
|
|
|
+
|
|
|
+ proto = PPP_PROTO(skb);
|
|
|
+- if (!pch->ppp || proto >= 0xc000 || proto == PPP_CCPFRAG) {
|
|
|
++ if (!ppp || proto >= 0xc000 || proto == PPP_CCPFRAG) {
|
|
|
+ /* put it on the channel queue */
|
|
|
+ skb_queue_tail(&pch->file.rq, skb);
|
|
|
+ /* drop old frames if queue too long */
|
|
|
+@@ -2315,11 +2318,11 @@ ppp_input(struct ppp_channel *chan, stru
|
|
|
+ kfree_skb(skb);
|
|
|
+ wake_up_interruptible(&pch->file.rwait);
|
|
|
+ } else {
|
|
|
+- ppp_do_recv(pch->ppp, skb, pch);
|
|
|
++ ppp_do_recv(ppp, skb, pch);
|
|
|
+ }
|
|
|
+
|
|
|
+ done:
|
|
|
+- read_unlock_bh(&pch->upl);
|
|
|
++ rcu_read_unlock_bh();
|
|
|
+ }
|
|
|
+
|
|
|
+ /* Put a 0-length skb in the receive queue as an error indication */
|
|
|
+@@ -2328,20 +2331,22 @@ ppp_input_error(struct ppp_channel *chan
|
|
|
+ {
|
|
|
+ struct channel *pch = chan->ppp;
|
|
|
+ struct sk_buff *skb;
|
|
|
++ struct ppp *ppp;
|
|
|
+
|
|
|
+ if (!pch)
|
|
|
+ return;
|
|
|
+
|
|
|
+- read_lock_bh(&pch->upl);
|
|
|
+- if (pch->ppp) {
|
|
|
++ rcu_read_lock_bh();
|
|
|
++ ppp = rcu_dereference_bh(pch->ppp);
|
|
|
++ if (ppp) {
|
|
|
+ skb = alloc_skb(0, GFP_ATOMIC);
|
|
|
+ if (skb) {
|
|
|
+ skb->len = 0; /* probably unnecessary */
|
|
|
+ skb->cb[0] = code;
|
|
|
+- ppp_do_recv(pch->ppp, skb, pch);
|
|
|
++ ppp_do_recv(ppp, skb, pch);
|
|
|
+ }
|
|
|
+ }
|
|
|
+- read_unlock_bh(&pch->upl);
|
|
|
++ rcu_read_unlock_bh();
|
|
|
+ }
|
|
|
+
|
|
|
+ /*
|
|
|
+@@ -2889,7 +2894,6 @@ int ppp_register_net_channel(struct net
|
|
|
+
|
|
|
+ pn = ppp_pernet(net);
|
|
|
+
|
|
|
+- pch->ppp = NULL;
|
|
|
+ pch->chan = chan;
|
|
|
+ pch->chan_net = get_net_track(net, &pch->ns_tracker, GFP_KERNEL);
|
|
|
+ chan->ppp = pch;
|
|
|
+@@ -2900,7 +2904,7 @@ int ppp_register_net_channel(struct net
|
|
|
+ #endif /* CONFIG_PPP_MULTILINK */
|
|
|
+ init_rwsem(&pch->chan_sem);
|
|
|
+ spin_lock_init(&pch->downl);
|
|
|
+- rwlock_init(&pch->upl);
|
|
|
++ spin_lock_init(&pch->upl);
|
|
|
+
|
|
|
+ spin_lock_bh(&pn->all_channels_lock);
|
|
|
+ pch->file.index = ++pn->last_channel_index;
|
|
|
+@@ -2929,13 +2933,15 @@ int ppp_channel_index(struct ppp_channel
|
|
|
+ int ppp_unit_number(struct ppp_channel *chan)
|
|
|
+ {
|
|
|
+ struct channel *pch = chan->ppp;
|
|
|
++ struct ppp *ppp;
|
|
|
+ int unit = -1;
|
|
|
+
|
|
|
+ if (pch) {
|
|
|
+- read_lock_bh(&pch->upl);
|
|
|
+- if (pch->ppp)
|
|
|
+- unit = pch->ppp->file.index;
|
|
|
+- read_unlock_bh(&pch->upl);
|
|
|
++ rcu_read_lock();
|
|
|
++ ppp = rcu_dereference(pch->ppp);
|
|
|
++ if (ppp)
|
|
|
++ unit = ppp->file.index;
|
|
|
++ rcu_read_unlock();
|
|
|
+ }
|
|
|
+ return unit;
|
|
|
+ }
|
|
|
+@@ -2947,12 +2953,14 @@ char *ppp_dev_name(struct ppp_channel *c
|
|
|
+ {
|
|
|
+ struct channel *pch = chan->ppp;
|
|
|
+ char *name = NULL;
|
|
|
++ struct ppp *ppp;
|
|
|
+
|
|
|
+ if (pch) {
|
|
|
+- read_lock_bh(&pch->upl);
|
|
|
+- if (pch->ppp && pch->ppp->dev)
|
|
|
+- name = pch->ppp->dev->name;
|
|
|
+- read_unlock_bh(&pch->upl);
|
|
|
++ rcu_read_lock();
|
|
|
++ ppp = rcu_dereference(pch->ppp);
|
|
|
++ if (ppp && ppp->dev)
|
|
|
++ name = ppp->dev->name;
|
|
|
++ rcu_read_unlock();
|
|
|
+ }
|
|
|
+ return name;
|
|
|
+ }
|
|
|
+@@ -3475,9 +3483,9 @@ ppp_connect_channel(struct channel *pch,
|
|
|
+ ppp = ppp_find_unit(pn, unit);
|
|
|
+ if (!ppp)
|
|
|
+ goto out;
|
|
|
+- write_lock_bh(&pch->upl);
|
|
|
++ spin_lock(&pch->upl);
|
|
|
+ ret = -EINVAL;
|
|
|
+- if (pch->ppp ||
|
|
|
++ if (rcu_dereference_protected(pch->ppp, lockdep_is_held(&pch->upl)) ||
|
|
|
+ rcu_dereference_protected(pch->bridge, lockdep_is_held(&pch->upl)))
|
|
|
+ goto outl;
|
|
|
+
|
|
|
+@@ -3502,13 +3510,13 @@ ppp_connect_channel(struct channel *pch,
|
|
|
+ ppp->dev->hard_header_len = hdrlen;
|
|
|
+ list_add_tail_rcu(&pch->clist, &ppp->channels);
|
|
|
+ ++ppp->n_channels;
|
|
|
+- pch->ppp = ppp;
|
|
|
++ rcu_assign_pointer(pch->ppp, ppp);
|
|
|
+ refcount_inc(&ppp->file.refcnt);
|
|
|
+ ppp_unlock(ppp);
|
|
|
+ ret = 0;
|
|
|
+
|
|
|
+ outl:
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+ out:
|
|
|
+ mutex_unlock(&pn->all_ppp_mutex);
|
|
|
+ return ret;
|
|
|
+@@ -3523,10 +3531,9 @@ ppp_disconnect_channel(struct channel *p
|
|
|
+ struct ppp *ppp;
|
|
|
+ int err = -EINVAL;
|
|
|
+
|
|
|
+- write_lock_bh(&pch->upl);
|
|
|
+- ppp = pch->ppp;
|
|
|
+- pch->ppp = NULL;
|
|
|
+- write_unlock_bh(&pch->upl);
|
|
|
++ spin_lock(&pch->upl);
|
|
|
++ ppp = rcu_replace_pointer(pch->ppp, NULL, lockdep_is_held(&pch->upl));
|
|
|
++ spin_unlock(&pch->upl);
|
|
|
+ if (ppp) {
|
|
|
+ /* remove it from the ppp unit's list */
|
|
|
+ ppp_lock(ppp);
|
Qingfang Deng