* [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
@ 2022-07-28 9:41 Fidaullah Noonari
2022-07-28 18:03 ` Dmitry Kozlyuk
0 siblings, 1 reply; 6+ messages in thread
From: Fidaullah Noonari @ 2022-07-28 9:41 UTC (permalink / raw)
To: dmitry.kozliuk; +Cc: dev, Fidaullah Noonari
The amount of memory to allocate from the system for heap expansion
was calculated in a way that may yield one page more than needed.
This could hit the allocation limit from the system or EAL.
The allocation would fail despite enough memory being available.
In response to mail:
http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
---
lib/eal/common/malloc_heap.c | 2 +-
lib/eal/common/malloc_mp.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 27a52266ad..d7c410b786 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -402,7 +402,7 @@ try_expand_heap_primary(struct malloc_heap *heap, uint64_t pg_sz,
int n_segs;
bool callback_triggered = false;
- alloc_sz = RTE_ALIGN_CEIL(align + elt_size +
+ alloc_sz = RTE_ALIGN_CEIL(RTE_ALIGN_CEIL(elt_size, align) +
MALLOC_ELEM_OVERHEAD, pg_sz);
n_segs = alloc_sz / pg_sz;
diff --git a/lib/eal/common/malloc_mp.c b/lib/eal/common/malloc_mp.c
index 2b8eb51067..15d85e2b2b 100644
--- a/lib/eal/common/malloc_mp.c
+++ b/lib/eal/common/malloc_mp.c
@@ -249,7 +249,7 @@ handle_alloc_request(const struct malloc_mp_req *m,
return -1;
}
- alloc_sz = RTE_ALIGN_CEIL(ar->align + ar->elt_size +
+ alloc_sz = RTE_ALIGN_CEIL(RTE_ALIGN_CEIL(ar->elt_size, ar->align) +
MALLOC_ELEM_OVERHEAD, ar->page_sz);
n_segs = alloc_sz / ar->page_sz;
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
2022-07-28 9:41 [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation Fidaullah Noonari
@ 2022-07-28 18:03 ` Dmitry Kozlyuk
2022-09-21 13:35 ` David Marchand
2022-09-26 9:20 ` David Marchand
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Kozlyuk @ 2022-07-28 18:03 UTC (permalink / raw)
To: Fidaullah Noonari; +Cc: dev
2022-07-28 14:41 (UTC+0500), Fidaullah Noonari:
> The amount of memory to allocate from the system for heap expansion
> was calculated in a way that may yield one page more than needed.
> This could hit the allocation limit from the system or EAL.
> The allocation would fail despite enough memory being available.
>
> In response to mail:
> http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
>
> Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
2022-07-28 18:03 ` Dmitry Kozlyuk
@ 2022-09-21 13:35 ` David Marchand
2022-09-21 22:52 ` Dmitry Kozlyuk
2022-09-26 9:20 ` David Marchand
1 sibling, 1 reply; 6+ messages in thread
From: David Marchand @ 2022-09-21 13:35 UTC (permalink / raw)
To: Dmitry Kozlyuk, Fidaullah Noonari; +Cc: dev, Burakov, Anatoly
On Thu, Jul 28, 2022 at 8:03 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> 2022-07-28 14:41 (UTC+0500), Fidaullah Noonari:
> > The amount of memory to allocate from the system for heap expansion
> > was calculated in a way that may yield one page more than needed.
> > This could hit the allocation limit from the system or EAL.
> > The allocation would fail despite enough memory being available.
> >
> > In response to mail:
> > http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
> >
> > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
>
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
>
Do you have a reproducer?
This sounds like a fix, should we backport it?
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
2022-09-21 13:35 ` David Marchand
@ 2022-09-21 22:52 ` Dmitry Kozlyuk
2022-09-26 9:18 ` David Marchand
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kozlyuk @ 2022-09-21 22:52 UTC (permalink / raw)
To: David Marchand; +Cc: Fidaullah Noonari, dev, Burakov, Anatoly
2022-09-21 15:35 (UTC+0200), David Marchand:
> On Thu, Jul 28, 2022 at 8:03 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> >
> > 2022-07-28 14:41 (UTC+0500), Fidaullah Noonari:
> > > The amount of memory to allocate from the system for heap expansion
> > > was calculated in a way that may yield one page more than needed.
> > > This could hit the allocation limit from the system or EAL.
> > > The allocation would fail despite enough memory being available.
> > >
> > > In response to mail:
> > > http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
> > >
> > > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> >
> > Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> >
>
> Do you have a reproducer?
> This sounds like a fix, should we backport it?
We should, thanks for the catch.
Reproducer:
#include <rte_eal.h>
#include <rte_malloc.h>
#include <stdio.h>
int
main(int argc, char **argv)
{
static const size_t hugepage_sz = 2 * (1 << 20);
static const size_t malloc_elem_overhead = 128;
static const size_t size = hugepage_sz - malloc_elem_overhead;
struct rte_malloc_socket_stats stats_before, stats_after;
size_t alloc_delta;
int ret;
ret = rte_eal_init(argc, argv);
if (ret < 0)
rte_panic("Cannot init EAL\n");
rte_malloc_get_socket_stats(0, &stats_before);
(void)rte_malloc(NULL, size, 0);
rte_malloc_get_socket_stats(0, &stats_after);
alloc_delta = stats_after.heap_totalsz_bytes - stats_before.heap_totalsz_bytes;
puts(alloc_delta == hugepage_sz ? "fixed" : "affected");
rte_eal_cleanup();
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
2022-09-21 22:52 ` Dmitry Kozlyuk
@ 2022-09-26 9:18 ` David Marchand
0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2022-09-26 9:18 UTC (permalink / raw)
To: Dmitry Kozlyuk; +Cc: Fidaullah Noonari, dev, Burakov, Anatoly
On Thu, Sep 22, 2022 at 12:52 AM Dmitry Kozlyuk
<dmitry.kozliuk@gmail.com> wrote:
>
> 2022-09-21 15:35 (UTC+0200), David Marchand:
> > On Thu, Jul 28, 2022 at 8:03 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> > >
> > > 2022-07-28 14:41 (UTC+0500), Fidaullah Noonari:
> > > > The amount of memory to allocate from the system for heap expansion
> > > > was calculated in a way that may yield one page more than needed.
> > > > This could hit the allocation limit from the system or EAL.
> > > > The allocation would fail despite enough memory being available.
> > > >
> > > > In response to mail:
> > > > http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
> > > >
> > > > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> > >
> > > Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > >
> >
> > Do you have a reproducer?
> > This sounds like a fix, should we backport it?
>
> We should, thanks for the catch.
>
> Reproducer:
>
> #include <rte_eal.h>
> #include <rte_malloc.h>
> #include <stdio.h>
>
> int
> main(int argc, char **argv)
> {
> static const size_t hugepage_sz = 2 * (1 << 20);
> static const size_t malloc_elem_overhead = 128;
> static const size_t size = hugepage_sz - malloc_elem_overhead;
> struct rte_malloc_socket_stats stats_before, stats_after;
> size_t alloc_delta;
> int ret;
>
> ret = rte_eal_init(argc, argv);
> if (ret < 0)
> rte_panic("Cannot init EAL\n");
> rte_malloc_get_socket_stats(0, &stats_before);
> (void)rte_malloc(NULL, size, 0);
> rte_malloc_get_socket_stats(0, &stats_after);
> alloc_delta = stats_after.heap_totalsz_bytes - stats_before.heap_totalsz_bytes;
> puts(alloc_delta == hugepage_sz ? "fixed" : "affected");
> rte_eal_cleanup();
> return 0;
Thanks Dmitry.
I'll put a note about this reproducer in the commitlog.
> }
>
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation
2022-07-28 18:03 ` Dmitry Kozlyuk
2022-09-21 13:35 ` David Marchand
@ 2022-09-26 9:20 ` David Marchand
1 sibling, 0 replies; 6+ messages in thread
From: David Marchand @ 2022-09-26 9:20 UTC (permalink / raw)
To: Fidaullah Noonari; +Cc: dev, Dmitry Kozlyuk, Burakov, Anatoly
On Thu, Jul 28, 2022 at 8:03 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> 2022-07-28 14:41 (UTC+0500), Fidaullah Noonari:
> > The amount of memory to allocate from the system for heap expansion
> > was calculated in a way that may yield one page more than needed.
> > This could hit the allocation limit from the system or EAL.
> > The allocation would fail despite enough memory being available.
> >
> > In response to mail:
> > http://inbox.dpdk.org/dev/CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com/
Cc: stable@dpdk.org
> >
> > Signed-off-by: Fidaullah Noonari <fidaullah.noonari@emumba.com>
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
I added a note about the reproducer (for testers) and applied.
Thanks!
--
David Marchand
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-26 9:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-28 9:41 [PATCH v2] eal: change alloc_sz calculation which may cause unnecessarily allocation Fidaullah Noonari
2022-07-28 18:03 ` Dmitry Kozlyuk
2022-09-21 13:35 ` David Marchand
2022-09-21 22:52 ` Dmitry Kozlyuk
2022-09-26 9:18 ` David Marchand
2022-09-26 9:20 ` David Marchand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).