All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: netdev@vger.kernel.org, linux-rt-devel@lists.linux.dev,
	linux-ppp@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Clark Williams <clrkwllms@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH net-next v2 1/1] ppp: Replace per-CPU recursion counter with lock-owner field
Date: Mon, 14 Jul 2025 18:10:47 +0200	[thread overview]
Message-ID: <aHUsB04j+uFrUkpd@debian> (raw)
In-Reply-To: <20250710162403.402739-2-bigeasy@linutronix.de>

On Thu, Jul 10, 2025 at 06:24:03PM +0200, Sebastian Andrzej Siewior wrote:
> The per-CPU variable ppp::xmit_recursion is protecting against recursion
> due to wrong configuration of the ppp channels. The per-CPU variable

I'd rather say that it's the ppp unit that is badly configured: it's
the ppp unit that can creates the loop (as it creates a networking
interface).

> relies on disabled BH for its locking. Without per-CPU locking in
> local_bh_disable() on PREEMPT_RT this data structure requires explicit
> locking.
> 
> The ppp::xmit_recursion is used as a per-CPU boolean. The counter is
> checked early in the send routing and the transmit path is only entered
> if the counter is zero. Then the counter is incremented to avoid
> recursion. It used to detect recursion on channel::downl and
> ppp::wlock.
> 
> Create a struct ppp_xmit_recursion and move the counter into it.
> Add local_lock_t to the struct and use local_lock_nested_bh() for
> locking. Due to possible nesting, the lock cannot be acquired
> unconditionally but it requires an owner field to identify recursion
> before attempting to acquire the lock.
> 
> The counter is incremented and checked only after the lock is acquired.
> Since it functions as a boolean rather than a count, and its role is now
> superseded by the owner field, it can be safely removed.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/net/ppp/ppp_generic.c | 38 ++++++++++++++++++++++++++---------
>  1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index def84e87e05b2..0edc916e0a411 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -119,6 +119,11 @@ struct ppp_link_stats {
>  	u64 tx_bytes;
>  };
>  
> +struct ppp_xmit_recursion {
> +	struct task_struct *owner;
> +	local_lock_t bh_lock;
> +};
> +

This hunk conflicts with latest changes in net-next.

Apart from the two minor comments above, the patch looks good to me.
Thanks!


  reply	other threads:[~2025-07-14 16:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-10 16:24 [PATCH net-next v2 0/1] ppp: Replace per-CPU recursion counter with lock-owner field Sebastian Andrzej Siewior
2025-07-10 16:24 ` [PATCH net-next v2 1/1] " Sebastian Andrzej Siewior
2025-07-14 16:10   ` Guillaume Nault [this message]
2025-07-14 20:01     ` Sebastian Andrzej Siewior
2025-07-15 17:33       ` Guillaume Nault

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aHUsB04j+uFrUkpd@debian \
    --to=gnault@redhat.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-ppp@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.