Re: [PATCH net-next v2 1/2] pppoe: remove rwlock usage

On Tue, Aug 26, 2025 at 7:31 PM Qingfang Deng [off-list ref] wrote:

Like ppp_generic.c, convert the PPPoE socket hash table to use RCU for
lookups and a spinlock for updates. This removes rwlock usage and allows
lockless readers on the fast path.

- Mark hash table and list pointers as __rcu.
- Use spin_lock() to protect writers.
- Readers use rcu_dereference() under rcu_read_lock(). All known callers
  of get_item() already hold the RCU read lock, so no additional locking
  is needed.
- get_item() now uses refcount_inc_not_zero() instead of sock_hold() to
  safely take a reference. This prevents crashes if a socket is already
  in the process of being freed (sk_refcnt == 0).
- Set SOCK_RCU_FREE to defer socket freeing until after an RCU grace
  period.

Signed-off-by: Qingfang Deng <dqfext@gmail.com>
---
v2:
 Use refcount_inc_not_zero() in get_item() to avoid taking a reference of
 a zero refcount socket.

Please next time include a pointer to other versions

  as in:
 v1 : https://lore.kernel.org/netdev/CALW65jZwrO5hQs_rm1Qo_+p-6yiKm+AdC9ZjkfjZnoWAm+i=Bg@mail.gmail.com/T/#m0c2d63508ec072f7a0079a8b22ddc35f622f051e (local)
 v2: https://lore.kernel.org/netdev/20250827023045.25002-1-dqfext@gmail.com/T/#t (local)

This allows reviewers to better follow the changes/suggestions.

I think there is one more problem with sockets destroying time.

sk->sk_destruct being the default (sock_def_destruct),
we will eventually leave some packets in sk->sk_receive_queue, and
kmemleak will fire.

You will need to add this part to make sure purge happens after RCU
grace period.

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 410effa42adef0f8dd2adec59dfe9f7d9f4a9339..763dea35fbcf4b30e09fb1e9c46386fdd9b5bc21

100644

--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c

@@ -528,6 +528,11 @@ static struct proto pppoe_sk_proto __read_mostly = {
        .obj_size = sizeof(struct pppox_sock),
 };

+static void pppoe_destruct(struct sock *sk)
+{
+       skb_queue_purge(&sk->sk_receive_queue);
+}
+
 /***********************************************************************
  *
  * Initialize a new struct sock.

@@ -542,6 +547,7 @@ static int pppoe_create(struct net *net, struct

socket *sock, int kern)
                return -ENOMEM;

        sock_init_data(sock, sk);
+       sk->sk_destruct = pppoe_destruct;

        sock->state     = SS_UNCONNECTED;
        sock->ops       = &pppoe_ops;

@@ -599,7 +605,6 @@ static int pppoe_release(struct socket *sock)
        sock_orphan(sk);
        sock->sk = NULL;

-       skb_queue_purge(&sk->sk_receive_queue);
        release_sock(sk);
        sock_put(sk);