[PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[Shakeel Butt]

Currently the initial allocation for pg_vec buffers are done through
page allocator with __GFP_NORETRY, the first fallbacks is vzalloc and
the second fallback is page allocator without __GFP_NORETRY.

First, there is no need to do vzalloc if the order is 0 and second the
vzalloc internally use GFP_KERNEL for each individual page allocation
and thus there is no need to have any fallback after vzalloc.

Signed-off-by: Shakeel Butt <shakeelb@google.com>
---
 net/packet/af_packet.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 29bd405adbbd..d6f96b9f5b01 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -4243,19 +4243,12 @@ static char *alloc_one_pg_vec_page(unsigned long order)
 	if (buffer)
 		return buffer;
 
-	/* __get_free_pages failed, fall back to vmalloc */
-	buffer = vzalloc(array_size((1 << order), PAGE_SIZE));
-	if (buffer)
-		return buffer;
+	/* __get_free_pages failed, fall back to vmalloc for high order. */
+	if (order)
+		return vzalloc(array_size((1 << order), PAGE_SIZE));
 
-	/* vmalloc failed, lets dig into swap here */
 	gfp_flags &= ~__GFP_NORETRY;
-	buffer = (char *) __get_free_pages(gfp_flags, order);
-	if (buffer)
-		return buffer;
-
-	/* complete and utter failure */
-	return NULL;
+	return (char *)__get_free_pages(gfp_flags, order);
 }
 
 static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order)
-- 
2.26.2.761.g0e0b3e54be-goog

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[David Miller]

From: Shakeel Butt <shakeelb@google.com>
Date: Fri, 15 May 2020 19:17:36 -0700

> and thus there is no need to have any fallback after vzalloc.

This statement is false.

The virtual mapping allocation or the page table allocations can fail.

A fallback is therefore indeed necessary.

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[Shakeel Butt]

On Sat, May 16, 2020 at 1:40 PM David Miller <davem@davemloft.net> wrote:
>
> From: Shakeel Butt <shakeelb@google.com>
> Date: Fri, 15 May 2020 19:17:36 -0700
>
> > and thus there is no need to have any fallback after vzalloc.
>
> This statement is false.
>
> The virtual mapping allocation or the page table allocations can fail.
>
> A fallback is therefore indeed necessary.

I am assuming that you at least agree that vzalloc should only be
called for non-zero order allocations. So, my argument is if non-zero
order vzalloc has failed (allocations internal to vzalloc, including
virtual mapping allocation and page table allocations, are order 0 and
use GFP_KERNEL i.e. triggering reclaim and oom-killer) then the next
non-zero order page allocation has very low chance of succeeding.

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[Eric Dumazet]

On Sat, May 16, 2020 at 3:35 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Sat, May 16, 2020 at 1:40 PM David Miller <davem@davemloft.net> wrote:
> >
> > From: Shakeel Butt <shakeelb@google.com>
> > Date: Fri, 15 May 2020 19:17:36 -0700
> >
> > > and thus there is no need to have any fallback after vzalloc.
> >
> > This statement is false.
> >
> > The virtual mapping allocation or the page table allocations can fail.
> >
> > A fallback is therefore indeed necessary.
>
> I am assuming that you at least agree that vzalloc should only be
> called for non-zero order allocations. So, my argument is if non-zero
> order vzalloc has failed (allocations internal to vzalloc, including
> virtual mapping allocation and page table allocations, are order 0 and
> use GFP_KERNEL i.e. triggering reclaim and oom-killer) then the next
> non-zero order page allocation has very low chance of succeeding.


32bit kernels might have exhausted their vmalloc space, yet they can
still allocate order-0 pages.

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[Shakeel Butt]

On Sat, May 16, 2020 at 3:45 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sat, May 16, 2020 at 3:35 PM Shakeel Butt <shakeelb@google.com> wrote:
> >
> > On Sat, May 16, 2020 at 1:40 PM David Miller <davem@davemloft.net> wrote:
> > >
> > > From: Shakeel Butt <shakeelb@google.com>
> > > Date: Fri, 15 May 2020 19:17:36 -0700
> > >
> > > > and thus there is no need to have any fallback after vzalloc.
> > >
> > > This statement is false.
> > >
> > > The virtual mapping allocation or the page table allocations can fail.
> > >
> > > A fallback is therefore indeed necessary.
> >
> > I am assuming that you at least agree that vzalloc should only be
> > called for non-zero order allocations. So, my argument is if non-zero
> > order vzalloc has failed (allocations internal to vzalloc, including
> > virtual mapping allocation and page table allocations, are order 0 and
> > use GFP_KERNEL i.e. triggering reclaim and oom-killer) then the next
> > non-zero order page allocation has very low chance of succeeding.
>
>
> 32bit kernels might have exhausted their vmalloc space, yet they can
> still allocate order-0 pages.

Oh ok it makes sense.

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[David Miller]

From: Shakeel Butt <shakeelb@google.com>
Date: Sat, 16 May 2020 15:35:46 -0700

> So, my argument is if non-zero order vzalloc has failed (allocations
> internal to vzalloc, including virtual mapping allocation and page
> table allocations, are order 0 and use GFP_KERNEL i.e. triggering
> reclaim and oom-killer) then the next non-zero order page allocation
> has very low chance of succeeding.

Also not true.

Page table allocation strategies and limits vary by architecture, they
may even need virtual mappings themselves.  So they can fail in situations
where a non-zero GFP_KERNEL page allocator allocation would succeed.

Re: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

[Shakeel Butt]

On Sat, May 16, 2020 at 4:39 PM David Miller <davem@davemloft.net> wrote:
>
> From: Shakeel Butt <shakeelb@google.com>
> Date: Sat, 16 May 2020 15:35:46 -0700
>
> > So, my argument is if non-zero order vzalloc has failed (allocations
> > internal to vzalloc, including virtual mapping allocation and page
> > table allocations, are order 0 and use GFP_KERNEL i.e. triggering
> > reclaim and oom-killer) then the next non-zero order page allocation
> > has very low chance of succeeding.
>
> Also not true.
>
> Page table allocation strategies and limits vary by architecture, they
> may even need virtual mappings themselves.  So they can fail in situations
> where a non-zero GFP_KERNEL page allocator allocation would succeed.

Thanks for the explanation. Do you think calling vzalloc only for
non-zero order here has any value?