Merge branch 'ppp-replace-per-cpu-recursion-counter-with-lock-owner-field'

Sebastian Andrzej Siewior says:

====================
ppp: Replace per-CPU recursion counter with lock-owner field

This is another approach to avoid relying on local_bh_disable() for
locking of per-CPU in ppp.

I redid it with the per-CPU lock and local_lock_nested_bh() as discussed
in v1. The xmit_recursion counter has been removed since it served the
same purpose as the owner field. Both were updated and checked.

The xmit_recursion looks like a counter in ppp_channel_push() but at
this point, the counter should always be 0 so it always serves as a
boolean. Therefore I removed it.

I do admit that this looks easier to review.

v2 https://lore.kernel.org/all/[email protected]/
v1 https://lore.kernel.org/all/[email protected]/
====================

Link: https://patch.msgid.link/[email protected]
Signed-off-by: Paolo Abeni <[email protected]>

Author: Paolo Abeni

drivers/net/ppp/ppp_generic.c

@@ -107,6 +107,11 @@ struct ppp_file {
 #define PF_TO_PPP(pf)		PF_TO_X(pf, struct ppp)
 #define PF_TO_CHANNEL(pf)	PF_TO_X(pf, struct channel)
 
+struct ppp_xmit_recursion {
+	struct task_struct *owner;
+	local_lock_t bh_lock;
+};
+
 /*
  * Data structure describing one ppp unit.
  * A ppp unit corresponds to a ppp network interface device
@@ -120,7 +125,7 @@ struct ppp {
 	int		n_channels;	/* how many channels are attached 54 */
 	spinlock_t	rlock;		/* lock for receive side 58 */
 	spinlock_t	wlock;		/* lock for transmit side 5c */
-	int __percpu	*xmit_recursion; /* xmit recursion detect */
+	struct ppp_xmit_recursion __percpu *xmit_recursion; /* xmit recursion detect */
 	int		mru;		/* max receive unit 60 */
 	unsigned int	flags;		/* control bits 64 */
 	unsigned int	xstate;		/* transmit state bits 68 */
@@ -1249,13 +1254,18 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev,
 	spin_lock_init(&ppp->rlock);
 	spin_lock_init(&ppp->wlock);
 
-	ppp->xmit_recursion = alloc_percpu(int);
+	ppp->xmit_recursion = alloc_percpu(struct ppp_xmit_recursion);
 	if (!ppp->xmit_recursion) {
 		err = -ENOMEM;
 		goto err1;
 	}
-	for_each_possible_cpu(cpu)
-		(*per_cpu_ptr(ppp->xmit_recursion, cpu)) = 0;
+	for_each_possible_cpu(cpu) {
+		struct ppp_xmit_recursion *xmit_recursion;
+
+		xmit_recursion = per_cpu_ptr(ppp->xmit_recursion, cpu);
+		xmit_recursion->owner = NULL;
+		local_lock_init(&xmit_recursion->bh_lock);
+	}
 
 #ifdef CONFIG_PPP_MULTILINK
 	ppp->minseq = -1;
@@ -1660,15 +1670,20 @@ static void __ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
 
 static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
 {
+	struct ppp_xmit_recursion *xmit_recursion;
+
 	local_bh_disable();
 
-	if (unlikely(*this_cpu_ptr(ppp->xmit_recursion)))
+	xmit_recursion = this_cpu_ptr(ppp->xmit_recursion);
+	if (xmit_recursion->owner == current)
 		goto err;
+	local_lock_nested_bh(&ppp->xmit_recursion->bh_lock);
+	xmit_recursion->owner = current;
 
-	(*this_cpu_ptr(ppp->xmit_recursion))++;
 	__ppp_xmit_process(ppp, skb);
-	(*this_cpu_ptr(ppp->xmit_recursion))--;
 
+	xmit_recursion->owner = NULL;
+	local_unlock_nested_bh(&ppp->xmit_recursion->bh_lock);
 	local_bh_enable();
 
 	return;
@@ -2169,11 +2184,16 @@ static void __ppp_channel_push(struct channel *pch)
 
 static void ppp_channel_push(struct channel *pch)
 {
+	struct ppp_xmit_recursion *xmit_recursion;
+
 	read_lock_bh(&pch->upl);
 	if (pch->ppp) {
-		(*this_cpu_ptr(pch->ppp->xmit_recursion))++;
+		xmit_recursion = this_cpu_ptr(pch->ppp->xmit_recursion);
+		local_lock_nested_bh(&pch->ppp->xmit_recursion->bh_lock);
+		xmit_recursion->owner = current;
 		__ppp_channel_push(pch);
-		(*this_cpu_ptr(pch->ppp->xmit_recursion))--;
+		xmit_recursion->owner = NULL;
+		local_unlock_nested_bh(&pch->ppp->xmit_recursion->bh_lock);
 	} else {
 		__ppp_channel_push(pch);
 	}