Can some memory providers not support the free() operation or not?

[ldorau]

The File Memory Provider and the DevDax Memory Provider currently do not support the free() operation.
They should be used together with the Coarse Memory Provider (#715) or with the jemalloc pool manager that can handle such situation.

It may seem weird that some memory providers do not support the free() operation. This issue is just for the discussion of this topic.

Ref: #759

See #759 for the beginning of this discussion: #759 (comment) :

Another question regarding correctness: when memory tracker is cleared? When the memory provider allocates a memory region it is added to the memory tracker, but if the pool implementation does not release memory back and relies on the fact that the memory provider implementation will release virtual memory mapping who will cleanup the memory tracker? If it is not done then it is an error, because after the memory provider is destroyed the virtual memory address range might be re-used by other memory providers and the same virtual memory region might be added to the tracker and it will fail because the same virtual memory region already presented in the tracker.

Right. So I replaced it with clearing the tracker for the current pool - please re-review

I still think that not supporting free operation in the memory provider is weird:

• First, it means that not every pool manager can be used with such memory provider or user needs to use coarse provider on top of upstream provider that does not support free operation.
• Second, in this PR you need to iterate via the tracker (that contains records from all pools) and filter only records for the corresponding pool. It might be a performance impact in real life.

Another consideration, I am working on the cache implementation for opened IPC handles, the cache is global and I need to remove entries from the IPC cache when the corresponding pool is destroyed. It looks like a similar problem. BUt I still think that in your case it is better to require free implementation in all providers.

@vinser52 @bratpiorka @pbalcer

[pbalcer]

For fsdax we probably could free pages on destroy (memkind does: https://github.com/memkind/memkind/blob/master/src/memkind_pmem.c#L159). The issue there is performance - deallocating such page is a very expensive operation (since, for fsdax, you are punching a hole in the filesystem) and it may not make sense if we can handle a situation where a provider does not support page deallocation.
So by forcing the provider to provide a suboptimal free implementation, we are leaving performance on the table.

For devdax, the situation is a bit different. It's a special case where not all mmap/munmap capabilities exist. For example, you need to align all your requests to the size with which the devdax region was created. This limitation might be tricky to adhere to for pool implementations, since you might want to be able to break down huge pages into smaller pages, and then merge them back again.
Memkind does not implement free for devdax "provider".

I guess we could only ever expose devdax/fsdax providers wrapped in a coarse provider. In that case, this would introduce an extraneous abstraction layer for scenarios that use fsdax provider and a jemalloc pool (which has builtin support for providers not support page deallocation).

Finally, we might also eventually create a provider that allocates out user-provided pages. Like fixed malloc in memkind: https://github.com/memkind/memkind/blob/master/examples/fixed_malloc.c#L31
This is basically the same thing as devdax, and we might not want to munmap pages mmaped by the user. Again, we can just wrap this in a coarse provider, but that adds overhead.

Second, in this PR you need to iterate via the tracker (that contains records from all pools) and filter only records for the corresponding pool. It might be a performance impact in real life.

Yes, but I'd argue this should be happening regardless of whether providers are allowed to not support free. Otherwise we risk bloating the tracker data structure if some piece of code does not properly clean up after itself.
And it only happens on provider destroy, so should be infrequent and not on the hot path. But I do a agree that having an O(n) operation isn't ideal. We probably should think of how we can use reduce this (with some reverse lookup?).

[ldorau]

See: #773