From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============1345935836102856257=="
MIME-Version: 1.0
From: Walker, Benjamin <benjamin.walker at intel.com>
Subject: Re: [SPDK] spdk_malloc vs. malloc
Date: Thu, 03 May 2018 23:24:31 +0000
Message-ID: <1525389870.46960.2.camel@intel.com>
In-Reply-To: 7B51D052-0906-4E4C-BE69-2EB587C95E27@netapp.com
List-ID: <spdk@lists.01.org>
To: spdk@lists.01.org

--===============1345935836102856257==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

On Thu, 2018-05-03 at 20:14 +0000, Meneghini, John wrote:
> The issue is: spdk_malloc and spdk_calloc are logical APIs to abstract the
> POSIX malloc and calloc calls.  We chose to NOT use those API names origi=
nally
> and went with the spdk_dma_{} api names. My concern is that taking those =
API
> names now will eliminate the possibility of any other use in the future.
> =

> I am not asking to "fully abstract POSIX".  I am simply saying that the
> malloc(), calloc() and free()  POSIX APIs suffer from some problems.  Thi=
s is
> why DPDK implemented rte_malloc() and rte_calloc().   As we move forward =
to
> build production quality products out of SPDK we will want to abstract ma=
lloc,
> calloc and free; especially in the libraries.  I don't care about what the
> applications do.  I want a better memory management APIs in the libraries.

DPDK implemented rte_malloc and rte_calloc to allocate DMA-safe and shared
memory. They still call POSIX malloc and calloc for all other types of
allocations. This is the same thing that SPDK has done.

I've never heard of malloc, calloc, and free having problems to the extent =
that
you'd want to reimplement them in your application some other way. Certainl=
y,
you may want to avoid calling them at run-time because they can fail, but S=
PDK
generally already does that for performance reasons anyway. Can you outline=
 what
you mean?

> =

> I'd be happy to simply map spdk_malloc/calloc() to rte_malloc/calloc() an=
d I
> don't see the point introducing an spdk_malloc() that only extends
> spdk_dma_malloc().  I'd rather absorb an API change to spdk_dma_malloc().=
  We
> change many APIs in SPDK from release to release. I don't see why this API
> can't be changed - with agreement from the SPDK community.
> =

> /*
>  * Allocate memory on default heap.
>  */
> void *
> rte_malloc(const char *type, size_t size, unsigned align)
> {
>         return rte_malloc_socket(type, size, align, SOCKET_ID_ANY);
> }
> =

> /*     =

>  * Allocate zero'd memory on default heap.
>  */
> void *
> rte_calloc(const char *type, size_t num, size_t size, unsigned align)
> {
>         return rte_zmalloc(type, num * size, align);
> }
> =

> /John
> =

> =EF=BB=BFOn 5/3/18, 3:21 PM, "Walker, Benjamin" <benjamin.walker(a)intel.=
com> wrote:
> =

>     On Thu, 2018-05-03 at 18:23 +0000, Meneghini, John wrote:
>     > >  If the user passes flags =3D=3D 0 to the new spdk_malloc() call,=
 this
> could be
>     > implemented by malloc() or equivalent behind the scenes,
>     >  =

>     > So, does this mean you=E2=80=99re willing to change all calls to ma=
lloc(size)
> with
>     > spdk_malloc(size, 0, NULL, SPDK_ENV_SOCKET_ID_ANY, SPDK_MALLOC_SHAR=
E)?
>     >  =

>     > Is that the plan?
>     > =

>     =

>     Hi John,
>         =

>     SPDK explicitly depends on POSIX throughout the code base. We do have=
 an
>     environment abstraction layer designed to make porting SPDK to various
>     environments (i.e. away from DPDK) easier, but this only abstracts
> operations
>     that are not available through standard POSIX calls. We're concerned =
that
> fully
>     abstracting POSIX would introduce a significant barrier to entry for =
new
>     contributors to the project, while only enabling one additional use c=
ase
> that
>     we're aware of. I understand that this one use case happens to be you=
rs,
> and so
>     this is a challenge for you.
>         =

>     In your code today, I assume you carry patches to replace the POSIX c=
alls
> with
>     your available alternatives. We've attempted to make carrying these
> patches
>     reasonably easy by moving all of the POSIX includes into a single hea=
der
>     (include/stdinc.h). Since you're already carrying those patches, can =
you
> simply
>     choose a different name for your replacement for malloc/calloc?
>     =

>     Thanks,
>     Ben
>     =

>=20

--===============1345935836102856257==--