Re: [PATCH mptcp-next] sock: add sock_krfree_s helper

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 30/04/2025 11:09, Geliang Tang wrote:
> This patch adds the rcu version of sock_kfree_s() helper, named
> sock_krfree_s(), to free the memory using kfree_rcu_mightsleep()
> and adjust the socket's option memory buffer.
> 
> This new helper is first used in MPTCP.
> 
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> ---
>  include/net/sock.h       |  1 +
>  net/core/sock.c          | 14 +++++++++++---
>  net/mptcp/pm_userspace.c |  3 +--
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index e223102337c7..94f72f5c8754 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1836,6 +1836,7 @@ void *sock_kmemdup(struct sock *sk, const void *src,
>  		   int size, gfp_t priority);
>  void sock_kfree_s(struct sock *sk, void *mem, int size);
>  void sock_kzfree_s(struct sock *sk, void *mem, int size);
> +void sock_krfree_s(struct sock *sk, void *mem, int size);
>  void sk_send_sigurg(struct sock *sk);
>  
>  static inline void sock_replace_proto(struct sock *sk, struct proto *proto)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index b64df2463300..f9ee0b44ab2a 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2862,12 +2862,14 @@ EXPORT_SYMBOL(sock_kmemdup);
>   * condition entirely.
>   */
>  static inline void __sock_kfree_s(struct sock *sk, void *mem, int size,
> -				  const bool nullify)
> +				  const bool nullify, const bool rcu)
>  {
>  	if (WARN_ON_ONCE(!mem))
>  		return;
>  	if (nullify)
>  		kfree_sensitive(mem);
> +	else if (rcu)
> +		kfree_rcu_mightsleep(mem);

I'm not sure about that: in some cases, kfree_rcu() might be called.
kfree_rcu_mightsleep() is more an exception to me, I think. Also, it
looks like there are many other places where the free and atomic_sub()
is done "manually", that's not specific to MPTCP.

I don't know what Mat thinks about that, but I don't think this is
needed (or it should replace some code in different places in the kernel).

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.