From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============1836239419884970941==" MIME-Version: 1.0 From: Harris, James R Subject: Re: [SPDK] spdk_malloc vs. malloc Date: Sat, 05 May 2018 00:17:12 +0000 Message-ID: In-Reply-To: CEDA3746-1240-4D86-9D2C-3966FF489B62@netapp.com List-ID: To: spdk@lists.01.org --===============1836239419884970941== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable You=E2=80=99re right Ed. We=E2=80=99ll need to hold off on adding usages o= f this new API until we get this sorted out. = What if we just add an spdk_dma_shared_malloc() and got rid of the flags co= mpletely? And then just keep all of the existing malloc/free calls where D= MA/shared memory is not required. This should alleviate the impact to your= code base and I *think* would work for Oracle =E2=80=93 but would need Zah= ra to confirm. This adds an explicit assumption of course that we won=E2= =80=99t have a need for some other kind of malloc =E2=80=98flag=E2=80=99 in= the future. Sorry John =E2=80=93 I know you wanted to end this thread but I=E2=80=99ve = decided to continue it. ( -Jim On 5/4/18, 3:37 PM, "Rodriguez, Edwin" wrote: The problem I have with the new code is that it makes distinguishing be= tween dma and non-dma free difficult. In the kernel, dma memory comes from= a different pool than regular allocations, i.e. contigmalloc/contigfree in= FreeBsd. How does spdk_free distinguish between a block allocated with SP= DK_MALLOC_DMA and a regular block? = Ed R = On 5/4/18, 6:30 PM, "SPDK on behalf of Meneghini, John" wrote: = Thanks, Ben, et al, for your reply. = > you may want to avoid calling them at run-time because they can f= ail, but SPDK generally already does > that for performance reasons anyway. Can you outline what you mea= n? = What is run-time? We are using the SPDK libraries inside of a ker= nel kmod. So, all time is run-time. = I think we can end this thread. I tried to win this argument once = before. I guess calloc() and malloc() are here to stay... so it doesn't matter what we do with spdk_malloc(). = :-) = If anyone wants to ask me about my views on this topic, I'll be at = the SPDK Developer's Summit in two weeks. = See you then. = /John = = On 5/3/18, 7:24 PM, "Walker, Benjamin" wrote: = 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 originally > and went with the spdk_dma_{} api names. My concern is that t= aking 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 sayin= g that the > malloc(), calloc() and free() POSIX APIs suffer from some pr= oblems. This is > why DPDK implemented rte_malloc() and rte_calloc(). As we m= ove forward to > build production quality products out of SPDK we will want to= abstract malloc, > calloc and free; especially in the libraries. I don't care a= bout what the > applications do. I want a better memory management APIs in t= he 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 t= ypes 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 w= ay. Certainly, you may want to avoid calling them at run-time because they can= fail, but SPDK 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() and I > don't see the point introducing an spdk_malloc() that only ex= tends > spdk_dma_malloc(). I'd rather absorb an API change to spdk_d= ma_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, unsigne= d align) > { > return rte_zmalloc(type, num * size, align); > } > = > /John > = > On 5/3/18, 3:21 PM, "Walker, Benjamin" 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_ma= lloc() 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 malloc(size) > with > > spdk_malloc(size, 0, NULL, SPDK_ENV_SOCKET_ID_ANY, SPDK= _MALLOC_SHARE)? > > = > > 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 SP= DK to various > environments (i.e. away from DPDK) easier, but this only = abstracts > operations > that are not available through standard POSIX calls. We'r= e concerned that > fully > abstracting POSIX would introduce a significant barrier t= o entry for new > contributors to the project, while only enabling one addi= tional use case > that > we're aware of. I understand that this one use case happe= ns to be yours, > and so > this is a challenge for you. > = > In your code today, I assume you carry patches to replace= the POSIX calls > with > your available alternatives. We've attempted to make carr= ying these > patches > reasonably easy by moving all of the POSIX includes into = a single header > (include/stdinc.h). Since you're already carrying those p= atches, can you > simply > choose a different name for your replacement for malloc/c= alloc? > = > Thanks, > Ben > = > = = = _______________________________________________ SPDK mailing list SPDK(a)lists.01.org https://lists.01.org/mailman/listinfo/spdk = = = --===============1836239419884970941==--