Re: [PATCH mptcp-next v1 5/6] sock: add sock_kmemdup helper

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 24/02/2025 09:13, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds the sock version of kmemdup() helper, named sock_kmemdup(),
> to duplicate a memory block using the socket's option memory buffer.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/sock.h |  1 +
>  net/core/sock.c    | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index edbb870e3f86..ffd757e7e329 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1793,6 +1793,7 @@ static inline struct sk_buff *sock_alloc_send_skb(struct sock *sk,
>  }
>  
>  void *sock_kmalloc(struct sock *sk, int size, gfp_t priority);
> +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 sk_send_sigurg(struct sock *sk);
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 0d385bf27b38..d09bd697c120 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2805,6 +2805,29 @@ void *sock_kmalloc(struct sock *sk, int size, gfp_t priority)
>  }
>  EXPORT_SYMBOL(sock_kmalloc);
>  
> +/*
> + * Duplicate a memory block using the socket's option memory buffer.
> + */
> +void *sock_kmemdup(struct sock *sk, const void *src, int size, gfp_t priority)
> +{
> +	int optmem_max = READ_ONCE(sock_net(sk)->core.sysctl_optmem_max);
> +
> +	if ((unsigned int)size <= optmem_max &&
> +	    atomic_read(&sk->sk_omem_alloc) + size < optmem_max) {
> +		void *mem;
> +		/* First do the add, to avoid the race if kmalloc
> +		 * might sleep.
> +		 */
> +		atomic_add(size, &sk->sk_omem_alloc);
> +		mem = kmemdup(src, size, priority);
> +		if (mem)
> +			return mem;
> +		atomic_sub(size, &sk->sk_omem_alloc);

I'm not really convinced by this helper: it is a duplication of
sock_kmalloc(), and it is only used once in MPTCP code.

Calling sock_kmalloc() + memset, and using this new helper in different
places in the net code might help. But still, I don't know if this would
be accepted, it is only saving one line (plus memcpy() will be used when
it is not needed, same for the previous patch at the end).

If you still want to propose that, I suggest sending a dedicated series
to netdev, not to block MPTCP (only) patches. WDYT?

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