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

[Paolo Abeni <pabeni@redhat.com>]

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;) 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.

/P