[PATCH] net: use non-compound pages in frag allocator

The code currently always references the first page in the
frag therefore there is no need to pay the extra overhead
of making the frag page compound

Signed-off-by: Ilya Lesokhin <redacted>
---
 net/core/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 08bf97e..77432ba 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -1993,7 +1993,7 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t gfp)
 	if (SKB_FRAG_PAGE_ORDER) {
 		/* Avoid direct reclaim but allow kswapd to wake */
 		pfrag->page = alloc_pages((gfp & ~__GFP_DIRECT_RECLAIM) |
-					  __GFP_COMP | __GFP_NOWARN |
+					  __GFP_NOWARN |
 					  __GFP_NORETRY,
 					  SKB_FRAG_PAGE_ORDER);
 		if (likely(pfrag->page)) {

-- 
1.8.3.1

On Thu, 2016-08-04 at 15:47 +0300, Ilya Lesokhin wrote:

Looks very risky to me.

What numbers can you show us exactly ?

splice() is known to play games on pages, make sure it still works ?

On Thu, 2016-08-04 at 13:38 +0000, Ilya Lesokhin wrote:

You are right I just realized that this patch causes a memory leak, so sorry for the spam.

I just had problem understanding when compound pages are needed and when It is enough to use an high order pages.
I thought that that high order pages are enough for this use case, tested it and saw no problems.

Unfortunately it seems you must use compound pages to keep track of  the page order.
put_page assumes that the order of a non-compound page is always 0. 
It would be nice to document it somewhere but I'm not sure where.

Please do not top-post on netdev mailing list.

( Documentation/development-process/2.Process : line 422 )

AFAIK, only when we can call free_pages(p, order) and provide @order, we
can avoid using __GFP_COMP at alloc time.

But if some path has to use put_page(), then we are forced to use
__GFP_COMP for high order pages.

Thanks.

You are right I just realized that this patch causes a memory leak, so sorry for the spam.

I just had problem understanding when compound pages are needed and when It is enough to use an high order pages.
I thought that that high order pages are enough for this use case, tested it and saw no problems.

Unfortunately it seems you must use compound pages to keep track of  the page order.
put_page assumes that the order of a non-compound page is always 0. 
It would be nice to document it somewhere but I'm not sure where.

Thanks,
Ilya

-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Thursday, August 04, 2016 4:25 PM
To: Ilya Lesokhin <redacted>
Cc: netdev@vger.kernel.org; edumazet@google.com
Subject: Re: [PATCH] net: use non-compound pages in frag allocator

On Thu, 2016-08-04 at 15:47 +0300, Ilya Lesokhin wrote:

quoted

The code currently always references the first page in the frag
therefore there is no need to pay the extra overhead of making the
frag page compound

Signed-off-by: Ilya Lesokhin <redacted>
---
 net/core/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c index 08bf97e..77432ba

100644

--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -1993,7 +1993,7 @@ bool skb_page_frag_refill(unsigned int sz, struct

page_frag *pfrag, gfp_t gfp)

quoted

 	if (SKB_FRAG_PAGE_ORDER) {
 		/* Avoid direct reclaim but allow kswapd to wake */
 		pfrag->page = alloc_pages((gfp & ~__GFP_DIRECT_RECLAIM)

|

quoted

-					  __GFP_COMP | __GFP_NOWARN |
+					  __GFP_NOWARN |
 					  __GFP_NORETRY,
 					  SKB_FRAG_PAGE_ORDER);
 		if (likely(pfrag->page)) {

Looks very risky to me.

What numbers can you show us exactly ?

splice() is known to play games on pages, make sure it still works ?