Nope, I think that's a valid approach. For some reason, I believe it
was problematic for the OpenIB guys to do that at the time we were
hacking up that code. But if it works, it sounds like a much better
approach.
When you make the change to the openib mpool, I'd also
MORECORE_CANNONT_TRIM back to 0. mvapi / openib were the only libraries
that needed the free in the deregistration callback -- GM apppeared to not
have that particular behavior. And I don't believe that anyone else
actually uses the deregistration callbacks.
Brian
On Mon, 7 Jan 2008, Gleb Natapov wrote:
Hi Brian,
I encountered problem with ptmalloc an registration cache. I see that
you (I think it was you) disabled shrinking of a heap memory allocated
by sbrk by setting MORECORE_CANNOT_TRIM to 1. The comment explains that
it should be done because freeing of small objects is not reentrant so
if ompi memory subsystem callback will call free() the code will deadlock.
And the trick indeed works in single threaded programs, but in multithreaded
programs ptmalloc may allocate a heap not only by sbrk, but by mmap too. This
is called "arena". Each thread may have arenas of its own. The problem is
that ptmalloc may free an arena by calling munmap() and then free() that
is called from our callback deadlocks. I tried to compile with USE_ARENAS set
to 0, but the code doesn't compile. I can fix compilation problem of
cause, but it seems that it is not so good idea to disable this feature.
The ptmalloc scalability depends on it (and even if we will disable it
ptmalloc may still create arena by mmap if sbrk fails). I see only one
way to solve this problem: to not call free() inside mpool callbacks.
If freeing of a memory is needed (and it is needed since IB unregister
calls free()) the works should be deferred. For IB mpool we can check what
needs to be unregistered inside a callback, but actually call unregister()
from next mpool->register() call. Do you see any problems with this
approach?
--
Gleb.
