From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============9043307963555400075==" MIME-Version: 1.0 From: Meneghini, John Subject: Re: [SPDK] spdk_malloc vs. malloc Date: Fri, 04 May 2018 22:29:56 +0000 Message-ID: <98BEACFD-D9CC-4157-AACF-3CE2AA3705E9@netapp.com> In-Reply-To: 1525389870.46960.2.camel@intel.com List-ID: To: spdk@lists.01.org --===============9043307963555400075== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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 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 = =EF=BB=BFOn 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 abstrac= t the > POSIX malloc and calloc calls. We chose to NOT use those API names o= riginally > and went with the spdk_dma_{} api names. My concern is that taking th= ose API > names now will eliminate the possibility of any other use in the futu= re. > = > I am not asking to "fully abstract POSIX". I am simply saying that t= he > malloc(), calloc() and free() POSIX APIs suffer from some problems. = This is > why DPDK implemented rte_malloc() and rte_calloc(). As we move forw= ard to > build production quality products out of SPDK we will want to abstrac= t malloc, > calloc and free; especially in the libraries. I don't care about wha= t the > applications do. I want a better memory management APIs in the libra= ries. = DPDK implemented rte_malloc and rte_calloc to allocate DMA-safe and sha= red 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 ext= ent that you'd want to reimplement them in your application some other way. Cert= ainly, you may want to avoid calling them at run-time because they can fail, b= ut SPDK generally already does that for performance reasons anyway. Can you out= line 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 extends > spdk_dma_malloc(). I'd rather absorb an API change to spdk_dma_mallo= c(). We > change many APIs in SPDK from release to release. I don't see why thi= s 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 > = > 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() c= all, this > could be > > implemented by malloc() or equivalent behind the scenes, > > = > > So, does this mean you=E2=80=99re willing to change all calls t= o 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 SPDK to va= rious > environments (i.e. away from DPDK) easier, but this only abstracts > operations > that are not available through standard POSIX calls. We're concer= ned that > fully > abstracting POSIX would introduce a significant barrier to entry = for new > contributors to the project, while only enabling one additional u= se case > that > we're aware of. I understand that this one use case happens to be= yours, > and so > this is a challenge for you. > = > In your code today, I assume you carry patches to replace the POS= IX calls > with > your available alternatives. We've attempted to make carrying the= se > patches > reasonably easy by moving all of the POSIX includes into a single= header > (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 > = > = = --===============9043307963555400075==--