From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id D9C95A0032;
Sat, 18 Jun 2022 13:29:48 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 7259B40F35;
Sat, 18 Jun 2022 13:29:48 +0200 (CEST)
Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com
[209.85.210.45]) by mails.dpdk.org (Postfix) with ESMTP id 540D440F19
for <dev@dpdk.org>; Sat, 18 Jun 2022 13:29:46 +0200 (CEST)
Received: by mail-ot1-f45.google.com with SMTP id
a21-20020a9d4715000000b0060bfaac6899so4900682otf.12
for <dev@dpdk.org>; Sat, 18 Jun 2022 04:29:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc; bh=DEgmsUq690Ygz5zVOpAMcjStbV6n1S7+FjC+VbHrdeE=;
b=MK8sR0E3Pcz1yKqo5xsxylK9C9b8RyT/ez8t+DJh9F3R6U7tJCUssLlj8QpKt0rROd
gTDSYIotwFe3esRF00AXOVO6LEbebt8q6DRaDNAb5WOsj25HC3HerctTAJj8zEa6s17L
y7S2qu2HCSnhm2KB8Kdilcib6eM2yF55tJxoi0Kk6JXfl45Oqsc6wMBxAvgrUlIrUbO/
KZAh++xhQWyGClCFA75MFnuTJZSGSFCpbq/BVflGniGgV0qIBUufqms7lwP3olLqJp21
iIeuO7KuC90Muvk5XiJdGkam496RX88QqSj/AFn3dmq/RnIkQ30AEr5F+QvcF/yhqyc3
UhOQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=DEgmsUq690Ygz5zVOpAMcjStbV6n1S7+FjC+VbHrdeE=;
b=WccPki6Wa5pShE65oi2hb47zoBD4wZ8AbuJMFRuzG8om++Kn1Kn+nQ3it35WH0rvtf
zwk0TBWSWHwIGpB5ehWCh4xfWzakMl3mZjiiXajr77HmdFLGnYN/cxPsKYe4LJKFBFp1
njBow+LGgdc/v9AbE9PbSWDg01kZnIPxJ7f3ZgW3DhQXp+2Jv5/XiH6JipbSy8aGCeO2
08iAzTUqi5CvHQrHHpvyDfJMMVSX1vdkNyfA1n5pwdka1ufcM1Sf8LbvuB8JeNWPQqea
BAA9OebrCXWedRMy2RA3Ldn/z5jSRBk8poezY4qLdjL+mFoCHXco/fzGN0Ozu1iIiWcx
1Pag==
X-Gm-Message-State: AJIora+DJ/S5HcY2x/2L1wdEuPrUWM1G66Os9ayPbTqbT0Vs7xtWXSXH
qDQ/re5M8GCct/g4GevIxWqXVsqgyTpAwxMAIrE=
X-Google-Smtp-Source: AGRyM1tGtMNRg8jesgu4qkM5LhR/7CYWLgFtBmrM0GtoaTUdaJ/pzdyuYB49P9uRDmReEffhvnNlkRYJc5V7iOZec0A=
X-Received: by 2002:a05:6830:2b25:b0:60c:221e:96ff with SMTP id
l37-20020a0568302b2500b0060c221e96ffmr5904199otv.78.1655551785615; Sat, 18
Jun 2022 04:29:45 -0700 (PDT)
MIME-Version: 1.0
References: <20220525051837.247255-1-fidaullah.noonari@emumba.com>
In-Reply-To: <20220525051837.247255-1-fidaullah.noonari@emumba.com>
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Date: Sat, 18 Jun 2022 14:29:34 +0300
Message-ID: <CAEYuUWCnRZNwxiOHEeTHw0Gy9aFJRLZtvAG9g=smuUvUEMcFXg@mail.gmail.com>
Subject: Re: [PATCH] eal: fixes the bug where rte_malloc() fails to allocates
memory
To: Fidaullah Noonari <fidaullah.noonari@emumba.com>
Cc: Anatoly Burakov <anatoly.burakov@intel.com>, dpdk-dev <dev@dpdk.org>
Content-Type: multipart/alternative; boundary="00000000000016c6dd05e1b73072"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
--00000000000016c6dd05e1b73072
Content-Type: text/plain; charset="UTF-8"
Hi Fidaullah,
Thanks for the fix,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Anatoly, I noticed a couple of other things while testing this.
1. Consider:
elt_size = pg_sz - MALLOC_ELEM_OVERHEAD
rte_malloc(align=0) which is converted to align = 1.
Obviously, such an element fits into one page, however:
alloc_sz = RTE_ALIGN_CEIL(1 + pg_sz +
(MALLOC_ELEM_OVERHEAD - MALLOC_ELEM_OVERHEAD),
pg_sz) == 2 * pg_sz.
This can unnecessarily hit an allocation limit from the system or EAL.
I suggest, in both places:
alloc_sz = RTE_ALIGN_CEIL(RTE_ALIGN_CEIL(elt_size, align) +
MALLOC_ELEM_OVERHEAD, pg_sz);
This would be symmetric with malloc_elem_can_hold().
2. Alignment calculation depends on whether we allocated new pages or not:
malloc_heap_alloc_on_heap_id(align = 0) ->
heap_alloc(align = 1) ->
find_suitable_element(align = RTE_CACHE_LINE_ROUNDUP(align))
malloc_heap_alloc_on_heap_id(align = 0) ->
alloc_more_mem_on_socket(align = 1) ->
try_expand_heap() -> ... ->
alloc_pages_on_heap(align = 1) ->
find_suitable_element(align = 1)
Why do we call find_suitable_element() directly and not just return
and repeat the heap_alloc() attempt?
--00000000000016c6dd05e1b73072
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div dir=3D"ltr"><div>Hi Fidaullah,</div><div><br></div><d=
iv>Thanks for the fix,<br></div><div>Acked-by: Dmitry Kozlyuk <<a href=
=3D"mailto:dmitry.kozliuk@gmail.com">dmitry.kozliuk@gmail.com</a>></div>=
<div><br></div><div><br></div><div>Anatoly, I noticed a couple of other thi=
ngs while testing this.<br></div><div><br></div><div>1. Consider:</div><div=
><br></div><div>elt_size =3D pg_sz - MALLOC_ELEM_OVERHEAD</div><div>rte_mal=
loc(align=3D0) which is converted to align =3D 1.</div><div><br></div><div>=
Obviously, such an element fits into one page, however:<br></div><div><br><=
/div><div>alloc_sz =3D RTE_ALIGN_CEIL(1 + pg_sz +</div><div>=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
(MALLOC_ELEM_OVERHEAD - MALLOC_ELEM_OVERHEAD),</div><div>=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
pg_sz) =3D=3D 2 * pg_sz.</div><div><br></div><div>This can unnecessarily h=
it an allocation limit from the system or EAL.<br></div><div>I suggest, in =
both places:<br></div><div><br> </div><div>alloc_sz =3D RTE_ALIGN_CEIL(RTE_=
ALIGN_CEIL(elt_size, align) +<br>=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 MALLOC_ELEM_OVERHEAD, p=
g_sz);</div><div><br></div><div>This would be symmetric with malloc_elem_ca=
n_hold().<br></div><div><br></div><div>2. Alignment calculation depends on =
whether we allocated new pages or not:<br></div><div><br></div><div>malloc_=
heap_alloc_on_heap_id(align =3D 0) -></div><div>heap_alloc(align =3D 1) =
-></div><div>find_suitable_element(align =3D RTE_CACHE_LINE_ROUNDUP(alig=
n))<br><br>malloc_heap_alloc_on_heap_id(align =3D 0) -><br>alloc_more_me=
m_on_socket(align =3D 1) -><br>try_expand_heap() -> ... -><br>allo=
c_pages_on_heap(align =3D 1) -><br>find_suitable_element(align =3D 1)<sp=
an style=3D"color:rgb(220,220,170)"></span></div><div><br></div></div><div>=
Why do we call find_suitable_element() directly and not just return</div><d=
iv>and repeat the heap_alloc() attempt?</div></div>
--00000000000016c6dd05e1b73072--