Commit a96cee9b authored by Paolo Abeni's avatar Paolo Abeni
Browse files

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/20250710162403.402739-1-bigeasy@linutronix.de/
v1 https://lore.kernel.org/all/20250627105013.Qtv54bEk@linutronix.de/
====================

Link: https://patch.msgid.link/20250715150806.700536-1-bigeasy@linutronix.de


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents e0c7e315 d4f6460a

drivers/net/ppp/ppp_generic.c

+29 −9
Original line number Diff line number Diff line
@@ -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);

	}