You’re right Ed. We’ll need to hold off on adding usages of 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 completely? And then just keep all of the existing malloc/free calls where DMA/shared memory is not required. This should alleviate the impact to your code base and I *think* would work for Oracle – but would need Zahra to confirm. This adds an explicit assumption of course that we won’t have a need for some other kind of malloc ‘flag’ in the future. Sorry John – I know you wanted to end this thread but I’ve 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 between 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 SPDK_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 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 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 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. 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 abstract 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 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. 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 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 > > 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 == 0 to the new spdk_malloc() call, this > could be > > implemented by malloc() or equivalent behind the scenes, > > > > So, does this mean you’re 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 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 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 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 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 > > _______________________________________________ SPDK mailing list SPDK(a)lists.01.org https://lists.01.org/mailman/listinfo/spdk