diff mbox series

[mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

Message ID bc6471c8f7444b89e7e5f6c8f7713972477fda36.1622045437.git.pabeni@redhat.com (mailing list archive)
State Accepted, archived
Commit fa92849db4ec571c6e3f6aa2efcf30e53a0c8d70
Delegated to: Matthieu Baerts
Headers show
Series [mptcp-net] mptcp: try harder to borrow memory from subflow under pressure | expand

Commit Message

Paolo Abeni May 26, 2021, 4:10 p.m. UTC
If the host is under sever memory pressure, and RX forward
memory allocation for the msk fails, we try to borrow the
required memory from the ingress subflow.

The current attempt is a bit flaky: if skb->truesize is less
than SK_MEM_QUANTUM, the ssk will not release any memory, and
the next schedule will fail again.

Instead, directly move the required amount of pages from the
ssk to the msk, if available

Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Mat Martineau May 27, 2021, 4:17 a.m. UTC | #1
On Wed, 26 May 2021, Paolo Abeni wrote:

> If the host is under sever memory pressure, and RX forward
> memory allocation for the msk fails, we try to borrow the
> required memory from the ingress subflow.
>
> The current attempt is a bit flaky: if skb->truesize is less
> than SK_MEM_QUANTUM, the ssk will not release any memory, and
> the next schedule will fail again.
>
> Instead, directly move the required amount of pages from the
> ssk to the msk, if available
>
> Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index bb029dd4ff5e..785e74c13b3c 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -286,11 +286,13 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
>
> 	/* try to fetch required memory from subflow */
> 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> -		if (ssk->sk_forward_alloc < skb->truesize)
> -			goto drop;
> -		__sk_mem_reclaim(ssk, skb->truesize);
> -		if (!sk_rmem_schedule(sk, skb, skb->truesize))
> +		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
> +
> +		if (ssk->sk_forward_alloc < amount)
> 			goto drop;
> +
> +		ssk->sk_forward_alloc -= amount;
> +		sk->sk_forward_alloc += amount;
> 	}
>
> 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> -- 
> 2.26.3

Looks ok to me - the memory will be reclaimed from the msk later, if I 
understand correctly.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel
Paolo Abeni May 27, 2021, 9:05 a.m. UTC | #2
On Wed, 2021-05-26 at 21:17 -0700, Mat Martineau wrote:
> On Wed, 26 May 2021, Paolo Abeni wrote:
> 
> > If the host is under sever memory pressure, and RX forward
> > memory allocation for the msk fails, we try to borrow the
> > required memory from the ingress subflow.
> > 
> > The current attempt is a bit flaky: if skb->truesize is less
> > than SK_MEM_QUANTUM, the ssk will not release any memory, and
> > the next schedule will fail again.
> > 
> > Instead, directly move the required amount of pages from the
> > ssk to the msk, if available
> > 
> > Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > net/mptcp/protocol.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index bb029dd4ff5e..785e74c13b3c 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -286,11 +286,13 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
> > 
> > 	/* try to fetch required memory from subflow */
> > 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > -		if (ssk->sk_forward_alloc < skb->truesize)
> > -			goto drop;
> > -		__sk_mem_reclaim(ssk, skb->truesize);
> > -		if (!sk_rmem_schedule(sk, skb, skb->truesize))
> > +		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
> > +
> > +		if (ssk->sk_forward_alloc < amount)
> > 			goto drop;
> > +
> > +		ssk->sk_forward_alloc -= amount;
> > +		sk->sk_forward_alloc += amount;
> > 	}
> > 
> > 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > -- 
> > 2.26.3
> 
> Looks ok to me - the memory will be reclaimed from the msk later, if I 
> understand correctly.

Yes, sk_forward_alloc will be updated when the skb will be freed at
reception time, and the fwd allocated memory is reclaimed either on
memory pressure or at socket close time. 
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thanks!

Paolo
Matthieu Baerts June 4, 2021, 7:34 p.m. UTC | #3
Hi Paolo, Mat,

On 26/05/2021 18:10, Paolo Abeni wrote:
> If the host is under sever memory pressure, and RX forward
> memory allocation for the msk fails, we try to borrow the
> required memory from the ingress subflow.
> 
> The current attempt is a bit flaky: if skb->truesize is less
> than SK_MEM_QUANTUM, the ssk will not release any memory, and
> the next schedule will fail again.
> 
> Instead, directly move the required amount of pages from the
> ssk to the msk, if available
> 
> Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the patch and the review! (and sorry for the delay)

- fa92849db4ec: mptcp: try harder to borrow memory from subflow under
pressure
- Results: 809268f427a5..119541ecd3c9

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210604T193342
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210604T193342

Cheers,
Matt
diff mbox series

Patch

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index bb029dd4ff5e..785e74c13b3c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -286,11 +286,13 @@  static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 
 	/* try to fetch required memory from subflow */
 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
-		if (ssk->sk_forward_alloc < skb->truesize)
-			goto drop;
-		__sk_mem_reclaim(ssk, skb->truesize);
-		if (!sk_rmem_schedule(sk, skb, skb->truesize))
+		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
+
+		if (ssk->sk_forward_alloc < amount)
 			goto drop;
+
+		ssk->sk_forward_alloc -= amount;
+		sk->sk_forward_alloc += amount;
 	}
 
 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;