2020-05-16 02:21:54

by Shakeel Butt

[permalink] [raw]
Subject: [PATCH] net/packet: simply allocations in alloc_one_pg_vec_page

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 <[email protected]>
---
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


2020-05-16 20:44:29

by David Miller

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

From: Shakeel Butt <[email protected]>
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.

2020-05-16 22:38:33

by Shakeel Butt

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

On Sat, May 16, 2020 at 1:40 PM David Miller <[email protected]> wrote:
>
> From: Shakeel Butt <[email protected]>
> 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.

2020-05-16 22:50:48

by Eric Dumazet

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

On Sat, May 16, 2020 at 3:35 PM Shakeel Butt <[email protected]> wrote:
>
> On Sat, May 16, 2020 at 1:40 PM David Miller <[email protected]> wrote:
> >
> > From: Shakeel Butt <[email protected]>
> > 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.

2020-05-16 23:41:18

by David Miller

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

From: Shakeel Butt <[email protected]>
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.

2020-05-16 23:54:48

by Shakeel Butt

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

On Sat, May 16, 2020 at 3:45 PM Eric Dumazet <[email protected]> wrote:
>
> On Sat, May 16, 2020 at 3:35 PM Shakeel Butt <[email protected]> wrote:
> >
> > On Sat, May 16, 2020 at 1:40 PM David Miller <[email protected]> wrote:
> > >
> > > From: Shakeel Butt <[email protected]>
> > > 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.

2020-05-17 00:01:20

by Shakeel Butt

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

On Sat, May 16, 2020 at 4:39 PM David Miller <[email protected]> wrote:
>
> From: Shakeel Butt <[email protected]>
> 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?