From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============6355524935000311868==" MIME-Version: 1.0 From: Rodriguez, Edwin Subject: Re: [SPDK] spdk_malloc vs. malloc Date: Fri, 04 May 2018 22:37:55 +0000 Message-ID: In-Reply-To: 98BEACFD-D9CC-4157-AACF-3CE2AA3705E9@netapp.com List-ID: To: spdk@lists.01.org --===============6355524935000311868== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable The problem I have with the new code is that it makes distinguishing betwee= n dma and non-dma free difficult. In the kernel, dma memory comes from a d= ifferent pool than regular allocations, i.e. contigmalloc/contigfree in Fre= eBsd. How does spdk_free distinguish between a block allocated with SPDK_M= ALLOC_DMA and a regular block? Ed R =EF=BB=BFOn 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 fail,= but SPDK generally already does > that for performance reasons anyway. Can you outline what you mean? = What is run-time? We are using the SPDK libraries inside of a kernel = kmod. So, all time is run-time. = I think we can end this thread. I tried to win this argument once befo= re. 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" wr= ote: = On Thu, 2018-05-03 at 20:14 +0000, Meneghini, John wrote: > The issue is: spdk_malloc and spdk_calloc are logical APIs to abs= tract the > POSIX malloc and calloc calls. We chose to NOT use those API nam= es originally > and went with the spdk_dma_{} api names. My concern is that takin= g 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 th= at the > malloc(), calloc() and free() POSIX APIs suffer from some proble= ms. This 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 abs= tract malloc, > 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 l= ibraries. = 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. = Certainly, you may want to avoid calling them at run-time because they can fai= l, 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/cal= loc() and 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_m= alloc(). 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 al= ign) > { > 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_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 cal= ls to malloc(size) > with > > spdk_malloc(size, 0, NULL, SPDK_ENV_SOCKET_ID_ANY, SPDK_MAL= LOC_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 SPDK t= o various > environments (i.e. away from DPDK) easier, but this only abst= racts > operations > that are not available through standard POSIX calls. We're co= ncerned that > fully > abstracting POSIX would introduce a significant barrier to en= try for new > contributors to the project, while only enabling one addition= al use case > that > we're aware of. I understand that this one use case happens t= o 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 carrying= these > patches > reasonably easy by moving all of the POSIX includes into a si= ngle header > (include/stdinc.h). Since you're already carrying those patch= es, can you > simply > choose a different name for your replacement for malloc/callo= c? > = > Thanks, > Ben > = > = = = _______________________________________________ SPDK mailing list SPDK(a)lists.01.org https://lists.01.org/mailman/listinfo/spdk = --===============6355524935000311868==--