skb: Propagate pfmemalloc on skb from head page only

PATCH In review Pavel Emelyanov 4 participants 7 messages Mar 14, 2013

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Pavel Emelyanov

author

2013-03-14 13:31 UTC #0 lore raw

PATCH 1/1 skb: Propagate pfmemalloc on skb from head page only

Hi.

I'm trying to send big chunks of memory from application address space via TCP socket using vmsplice + splice like this

mem = mmap(128Mb);
vmsplice(pipe[1], mem); /* splice memory into pipe */ splice(pipe[0], tcp_socket); /* send it into network */

When I'm lucky and a huge page splices into the pipe and then into the socket _and_ client and server ends of the TCP connection are on the same host, communicating via lo, the whole connection gets stuck! The sending queue becomes full and app stops writing/splicing more into it, but the receiving queue remains empty, and that's why.

The __skb_fill_page_desc observes a tail page of a huge page and erroneously propagates its page->pfmemalloc value onto socket (the pfmemalloc on tail pages contain garbage). Then this skb->pfmemalloc leaks through lo and due to the

    tcp_v4_rcv
    sk_filter
        if (skb->pfmemalloc && !sock_flag(sk, SOCK_MEMALLOC)) /* true */
            return -ENOMEM
        goto release_and_discard;

no packets reach the socket. Even TCP re-transmits are dropped by this, as skb cloning clones the pfmemalloc flag as well.

That said, here's the proper page->pfmemalloc propagation onto socket: we must check the huge-page's head page only, other pages' pfmemalloc and mapping values do not contain what is expected in this place. However, I'm not sure whether this fix is _complete_, since pfmemalloc propagation via lo also oesn't look great.

Both, bit propagation from page to skb and this check in sk_filter, were introduced by c48a11c7 (netvm: propagate page->pfmemalloc to skb), in v3.5 so Mel and stable@ are in Cc.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/linux/skbuff.h +4 -2

index eb2106f..4e525eb 100644
@@ -1292,11 +1292,13 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
 	 * do not lose pfmemalloc information as the pages would not be
 	 * allocated using __GFP_MEMALLOC.
 	 */
-	if (page->pfmemalloc && !page->mapping)
-		skb->pfmemalloc	= true;
 	frag->page.p		  = page;
 	frag->page_offset	  = off;
 	skb_frag_size_set(frag, size);
+
+	page = compound_head(page);
+	if (page->pfmemalloc && !page->mapping)
+		skb->pfmemalloc	= true;
 }
 
 /**

Eric Dumazet

Acked-by on v1 1/1

2013-03-14 14:16 UTC #1 lore raw

Quoted 53 lines · Pavel Emelyanov

Hi.
I'm trying to send big chunks of memory from application address space via TCP socket using vmsplice + splice like this
mem = mmap(128Mb);
vmsplice(pipe[1], mem); /* splice memory into pipe */ splice(pipe[0], tcp_socket); /* send it into network */
When I'm lucky and a huge page splices into the pipe and then into the socket _and_ client and server ends of the TCP connection are on the same host, communicating via lo, the whole connection gets stuck! The sending queue becomes full and app stops writing/splicing more into it, but the receiving queue remains empty, and that's why.
The __skb_fill_page_desc observes a tail page of a huge page and erroneously propagates its page->pfmemalloc value onto socket (the pfmemalloc on tail pages contain garbage). Then this skb->pfmemalloc leaks through lo and due to the
    tcp_v4_rcv
    sk_filter
        if (skb->pfmemalloc && !sock_flag(sk, SOCK_MEMALLOC)) /* true */
            return -ENOMEM
        goto release_and_discard;
no packets reach the socket. Even TCP re-transmits are dropped by this, as skb cloning clones the pfmemalloc flag as well.
That said, here's the proper page->pfmemalloc propagation onto socket: we must check the huge-page's head page only, other pages' pfmemalloc and mapping values do not contain what is expected in this place. However, I'm not sure whether this fix is _complete_, since pfmemalloc propagation via lo also oesn't look great.
Both, bit propagation from page to skb and this check in sk_filter, were introduced by c48a11c7 (netvm: propagate page->pfmemalloc to skb), in v3.5 so Mel and stable@ are in Cc.
Signed-off-by Pavel Emelyanov <[email protected]>
---
include/linux/skbuff.h +0 -2
index eb2106f..4e525eb 100644
@@ -1292,11 +1292,13 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
 	 * do not lose pfmemalloc information as the pages would not be
 	 * allocated using __GFP_MEMALLOC.
 	 */
-	if (page->pfmemalloc && !page->mapping)
-		skb->pfmemalloc	= true;
 	frag->page.p		  = page;

include/linux/skbuff.h static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,

Quoted 10 lines

include/linux/skbuff.h +4 -1

 	frag->page_offset	  = off;
 	skb_frag_size_set(frag, size);
+
+	page = compound_head(page);
+	if (page->pfmemalloc && !page->mapping)
+		skb->pfmemalloc	= true;
 }
 
 /**
--

This looks a nice finding.

Note this can trigger even without vmsplice() use but regular network receive.

Acked-by Eric Dumazet <[email protected]>

When I discussed with David on this issue, I said that one possibility would be to accept a pfmemalloc skb on regular skb if no other packet is in a receive queue, to get a chance to make progress (and limit memory consumption to no more than one skb per TCP socket)

Pavel Emelyanov

author

2013-03-14 14:25 UTC #2 lore raw

Quoted 33 lines

Quoted 33 lines
That said, here's the proper page->pfmemalloc propagation onto socket: we must check the huge-page's head page only, other pages' pfmemalloc and mapping values do not contain what is expected in this place. However, I'm not sure whether this fix is _complete_, since pfmemalloc propagation via lo also oesn't look great.
Both, bit propagation from page to skb and this check in sk_filter, were introduced by c48a11c7 (netvm: propagate page->pfmemalloc to skb), in v3.5 so Mel and stable@ are in Cc.
Signed-off-by Pavel Emelyanov <[email protected]>
---
include/linux/skbuff.h +4 -2
index eb2106f..4e525eb 100644
@@ -1292,11 +1292,13 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
 	 * do not lose pfmemalloc information as the pages would not be
 	 * allocated using __GFP_MEMALLOC.
 	 */
-	if (page->pfmemalloc && !page->mapping)
-		skb->pfmemalloc	= true;
 	frag->page.p		  = page;
 	frag->page_offset	  = off;
 	skb_frag_size_set(frag, size);
+
+	page = compound_head(page);
+	if (page->pfmemalloc && !page->mapping)
+		skb->pfmemalloc	= true;
 }
 

Quoted 7 lines

include/linux/skbuff.h static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
Quoted 2 lines
include/linux/skbuff.h +0 -1
 /**
--
This looks a nice finding.
Note this can trigger even without vmsplice() use but regular network receive.

Presumably you're right, but I don't understand how :( In order to trigger this, we should have a huge page, that gets linked to an skb _before_ it enters the TCP receive path. How can this happen when doing sendmsg/recvmsg?

Quoted 6 lines

Acked-by Eric Dumazet <[email protected]>
When I discussed with David on this issue, I said that one possibility would be to accept a pfmemalloc skb on regular skb if no other packet is in a receive queue, to get a chance to make progress (and limit memory consumption to no more than one skb per TCP socket)

Thanks,
Pavel

Eric Dumazet

2013-03-14 14:34 UTC #3 lore raw

Quoted 3 lines · Pavel Emelyanov

Presumably you're right, but I don't understand how :( In order to trigger this, we should have a huge page, that gets linked to an skb _before_ it enters the TCP receive path. How can this happen when doing sendmsg/recvmsg?

Not only huge pages.

network now uses order-3 pages in both transmit and receive paths.

Pavel Emelyanov

author

2013-03-14 14:37 UTC #4 lore raw

Quoted 9 lines · Eric Dumazet

Quoted 3 lines · Pavel Emelyanov
Presumably you're right, but I don't understand how :( In order to trigger this, we should have a huge page, that gets linked to an skb _before_ it enters the TCP receive path. How can this happen when doing sendmsg/recvmsg?
Not only huge pages.
network now uses order-3 pages in both transmit and receive paths.

Ah, indeed! Thanks, Eric.

David Miller

maintainer

2013-03-14 15:54 UTC #5 lore raw

In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Thu, 14 Mar 2013 15:16:09 +0100

Quoted 40 lines · Pavel Emelyanov

Quoted 39 lines
Hi.
I'm trying to send big chunks of memory from application address space via TCP socket using vmsplice + splice like this
mem = mmap(128Mb);
vmsplice(pipe[1], mem); /* splice memory into pipe */ splice(pipe[0], tcp_socket); /* send it into network */
When I'm lucky and a huge page splices into the pipe and then into the socket _and_ client and server ends of the TCP connection are on the same host, communicating via lo, the whole connection gets stuck! The sending queue becomes full and app stops writing/splicing more into it, but the receiving queue remains empty, and that's why.
The __skb_fill_page_desc observes a tail page of a huge page and erroneously propagates its page->pfmemalloc value onto socket (the pfmemalloc on tail pages contain garbage). Then this skb->pfmemalloc leaks through lo and due to the
    tcp_v4_rcv
    sk_filter
        if (skb->pfmemalloc && !sock_flag(sk, SOCK_MEMALLOC)) /* true */
            return -ENOMEM
        goto release_and_discard;
no packets reach the socket. Even TCP re-transmits are dropped by this, as skb cloning clones the pfmemalloc flag as well.
That said, here's the proper page->pfmemalloc propagation onto socket: we must check the huge-page's head page only, other pages' pfmemalloc and mapping values do not contain what is expected in this place. However, I'm not sure whether this fix is _complete_, since pfmemalloc propagation via lo also oesn't look great.
Both, bit propagation from page to skb and this check in sk_filter, were introduced by c48a11c7 (netvm: propagate page->pfmemalloc to skb), in v3.5 so Mel and stable@ are in Cc.
Signed-off-by Pavel Emelyanov <[email protected]>

...

Quoted 1 line

Acked-by Eric Dumazet <[email protected]>

Applied and queued up for -stable, thanks.

Mel Gorman

Acked-by on v1 1/1

2013-03-14 14:28 UTC #6 lore raw

In reply to Pavel Emelyanov

Quoted 34 lines · Pavel Emelyanov

Hi.
I'm trying to send big chunks of memory from application address space via TCP socket using vmsplice + splice like this
mem = mmap(128Mb);
vmsplice(pipe[1], mem); /* splice memory into pipe */ splice(pipe[0], tcp_socket); /* send it into network */
When I'm lucky and a huge page splices into the pipe and then into the socket _and_ client and server ends of the TCP connection are on the same host, communicating via lo, the whole connection gets stuck! The sending queue becomes full and app stops writing/splicing more into it, but the receiving queue remains empty, and that's why.
The __skb_fill_page_desc observes a tail page of a huge page and erroneously propagates its page->pfmemalloc value onto socket (the pfmemalloc on tail pages contain garbage). Then this skb->pfmemalloc leaks through lo and due to the
    tcp_v4_rcv
    sk_filter
        if (skb->pfmemalloc && !sock_flag(sk, SOCK_MEMALLOC)) /* true */
            return -ENOMEM
        goto release_and_discard;
no packets reach the socket. Even TCP re-transmits are dropped by this, as skb cloning clones the pfmemalloc flag as well.
That said, here's the proper page->pfmemalloc propagation onto socket: we must check the huge-page's head page only, other pages' pfmemalloc and mapping values do not contain what is expected in this place. However, I'm not sure whether this fix is _complete_, since pfmemalloc propagation via lo also oesn't look great.

Quoted 6 lines

Both, bit propagation from page to skb and this check in sk_filter, were introduced by c48a11c7 (netvm: propagate page->pfmemalloc to skb), in v3.5 so Mel and stable@ are in Cc.
Signed-off-by Pavel Emelyanov <[email protected]>

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs