Re: [PATCH mptcp-net 1/3] net: factor-out _sk_charge() helper

[Matthieu Baerts <matttbe@kernel.org>]

On 12/11/2025 10:28, Paolo Abeni wrote:
> On 11/12/25 10:16 AM, Matthieu Baerts wrote:
>> On 12/11/2025 10:04, Paolo Abeni wrote:
>>>
>>> On 11/11/25 6:38 PM, Matthieu Baerts wrote:
>>>> On 07/11/2025 22:55, Paolo Abeni wrote:
>>>>> Move out of __inet_accept() the code dealing charging newly
>>>>> accepted socket to memcg. MPTCP will soon use it to on a per
>>>>> subflow basis, in different contexts.
>>>>>
>>>>> No functional changes intended.
>>
>> (...)
>>
>>>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>>>> index a31b94ce8968..08d811f11896 100644
>>>>> --- a/net/ipv4/af_inet.c
>>>>> +++ b/net/ipv4/af_inet.c
>>>>> @@ -756,23 +756,8 @@ EXPORT_SYMBOL(inet_stream_connect);
>>>>>  void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *newsk)
>>>>>  {
>>>>>  	if (mem_cgroup_sockets_enabled) {
>>>>> -		gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
>>>>> -
>>>>>  		mem_cgroup_sk_alloc(newsk);
>>>>> -
>>>>> -		if (mem_cgroup_from_sk(newsk)) {
>>>>> -			int amt;
>>>>> -
>>>>> -			/* The socket has not been accepted yet, no need
>>>>> -			 * to look at newsk->sk_wmem_queued.
>>>>> -			 */
>>>>> -			amt = sk_mem_pages(newsk->sk_forward_alloc +
>>>>> -					   atomic_read(&newsk->sk_rmem_alloc));
>>>>> -			if (amt)
>>>>> -				mem_cgroup_sk_charge(newsk, amt, gfp);
>>>>> -		}
>>>>> -
>>>>> -		kmem_cache_charge(newsk, gfp);
>>>>
>>>> Mmh, this code has been moved from inet_csk_accept() to __inet_accept()
>>>> in net-next only, see commit 4a997d49d92a ("tcp: Save lock_sock() for
>>>> memcg in inet_csk_accept()."):
>>>>
>>>>   https://lore.kernel.org/20251014235604.3057003-2-kuniyu@google.com
>>>
>>> Right you are!
>>>
>>>> Should we only apply these patches in our export branch, for net-next?
>>>> If yes, then I guess we should remove the Fixes tag in patch 3/3.
>>>
>>> Technically is a fix. Even prior to the backlog introduction bad thing
>>> could happen, as passive msk does not have memory by the subflow only
>>> accounted.
>>>
>>> TCP-level OoO could potentially use a lot of system memory that will not
>>> be memaccounted.
>>>
>>> Unfortunately it's not easy to have a clean net patch.
>>>
>>> I suppose with can have this as net-next fixes (including the fixes tag
>>> in 3/3) as the change is invasive and the thing is broken since the
>>> beginning.
>>>
>>> WDYT?
>>
>> It sounds good to me. When these patches will be in Linus tree, we can
>> ask to backport these 3 patches from this series, plus commit
>> 4a997d49d92a ("tcp: Save lock_sock() for memcg in inet_csk_accept()."), no?
>>
>> If yes, do you think I can add a "Cc: stable", and explaining in the
>> comments that it is because it depends on a patch that is only in
>> net-next, and the fix is not urgent?
> 
> You know I don't have all the fascination for stable;)

I forgot :)

> My first take
> would be to avoid such tag. If you are willing to do the extra mile I
> guess that is a good plan. Alternatively you could just send the patch
> to stable after merge, with pre-req included.

Adding the "Cc: stable" allow me to forget about backporting it, except
if there are conflicts! But I can add a reminder :)

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