Re: [PATCH mptcp-next v2] Squash-to "mptcp: pm: init and release mptcp_pm_ops"

[Geliang Tang <geliang@kernel.org>]

Hi Gang,

Thanks for this v2.

On Wed, 2026-07-01 at 15:37 +0800, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> This patch removes the need to handle a NULL pm_ops in the fallback
> path, and moves the rcu_read_lock\unlock into mptcp_pm_ops_init.
> 
> Following sashiko's comments, it also moves the pr_debug() before
> bpf_module_put() in mptcp_pm_ops_release.
> 
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  net/mptcp/pm.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 9dc7b41fb562..5fe4276cf895 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1137,12 +1137,14 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
>  	spin_unlock_bh(&msk->pm.lock);
>  }
>  
> -static void mptcp_pm_ops_init(struct mptcp_sock *msk,
> -			      struct mptcp_pm_ops *pm_ops)
> +static void mptcp_pm_ops_init(struct mptcp_sock *msk, const char
> *pm_name)
>  {
> +	struct mptcp_pm_ops *pm_ops;
> +
> +	rcu_read_lock();
> +	pm_ops = mptcp_pm_find(pm_name);
>  	if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
> -		pr_warn_once("pm %s fails, fallback to default pm",
> -			     pm_ops->name);
> +		pr_warn_once("pm %s fails, fallback to default pm",
> pm_name);
>  		pm_ops = &mptcp_pm_kernel;
>  	}
>  
> @@ -1151,6 +1153,7 @@ static void mptcp_pm_ops_init(struct mptcp_sock
> *msk,
>  		msk->pm.ops->init(msk);
>  
>  	pr_debug("pm %s initialized\n", pm_ops->name);

nit: Here we can also print "pm_name" to keep it consistent with
pr_warn_once().

> +	rcu_read_unlock();

Then this unlock line can be moved ahead of the previous pr_debug()
line.

>  }
>  
>  static void mptcp_pm_ops_release(struct mptcp_sock *msk)
> @@ -1161,9 +1164,9 @@ static void mptcp_pm_ops_release(struct
> mptcp_sock *msk)
>  	if (pm_ops->release)
>  		pm_ops->release(msk);
>  
> -	bpf_module_put(pm_ops, pm_ops->owner);
> -
>  	pr_debug("pm %s released\n", pm_ops->name);
> +
> +	bpf_module_put(pm_ops, pm_ops->owner);
>  }
>  
>  void mptcp_pm_destroy(struct mptcp_sock *msk)
> @@ -1184,9 +1187,7 @@ void mptcp_pm_data_reset(struct mptcp_sock
> *msk)
>  	pm->rm_list_rx.nr = 0;
>  	WRITE_ONCE(pm->pm_type, pm_type);
>  
> -	rcu_read_lock();
> -	mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
> -	rcu_read_unlock();

Or we leave rcu_read_lock() and rcu_read_unlock() where they are,
without moving them into mptcp_pm_ops_init().

Thanks,
-Geliang

> +	mptcp_pm_ops_init(msk, pm_name);
>  }
>  
>  void mptcp_pm_data_init(struct mptcp_sock *msk)