mpool/rcache rewrite #1118

hjelmn · 2015-11-10T17:10:37Z

This PR brings in a complete rewrite of the mpool and rcache frameworks.

@vvenkates27 please verify memkind usability.

hjelmn · 2015-11-10T18:45:05Z

@jsquyres, @bosilca, @hppritcha Comments on the new interfaces are welcome. Let me know soon if you see any problems or things that can be improved. This is targeted for 2.0.0.

vvenkates27 · 2015-11-11T03:04:47Z

Hi Nathan,

Thanks for putting this together. I tested this out. It worked for the MPI_Alloc_mem.
MPI_Free_mem was not picking up right mpool module because the allocated address was not added to the rb-tree maintained in the base module in mca_mpool_base_alloc.c:99. "mem" needs to be added as the key to the rb-tree. Doing that fixes that issue.

I will let you know if I find any other issues.

Thanks
Vish

hjelmn · 2015-11-11T15:36:25Z

Fixed the red-black tree issue. Still trying to track down the mellanox issue.

hjelmn · 2015-11-23T23:59:01Z

:bot:retest

Can't reproduce the Mellanox Jenkins failure.

hjelmn · 2015-11-23T23:59:46Z

:bot:retest:

hjelmn · 2015-11-24T19:27:24Z

PR got screwed up on rebase. Fixing.

jsquyres · 2015-11-30T17:17:41Z

@hjelmn Any idea what's going wrong at btl_openib_endpoint.c:356?

hppritcha · 2015-11-30T17:35:44Z

Assuming this isn't addressing a bug in the existing mpool/rcache implementation, I think this is too large a change to go in to 2.0 at this point. It looks like a good candidate for 2.1.

hjelmn · 2015-12-09T22:03:10Z

Should be working now. @bosilca, @jsquyres Any comments on the new interfaces?

hjelmn · 2015-12-09T23:15:34Z

@miked-mellanox I can't tell what in the Mellanox Jenkins failed.

hjelmn · 2015-12-09T23:40:33Z

:bot:retest:

hjelmn · 2015-12-11T01:37:32Z

:bot:retest:

hjelmn · 2015-12-15T17:53:18Z

:bot:retest:

hjelmn · 2015-12-17T17:53:54Z

@miked-mellanox This looks like a false failure. When I run your env_mpi by hand with valgrind I get this:

==12372== Invalid read of size 8
==12372==    at 0x40082D: main (in /lustre/scratch1/yellow/hjelmn/ibm/env_mpi)
==12372==  Address 0xe132510 is 0 bytes inside a block of size 960 free'd
==12372==    at 0x4C27C20: realloc (vg_replace_malloc.c:662)
==12372==    by 0x56AD3FC: __add_to_environ (setenv.c:143)
==12372==    by 0x4C2B679: setenv (mc_replace_strmem.c:1756)
==12372==    by 0x50A2E34: ompi_mtl_mxm_component_open (mtl_mxm_component.c:216)
==12372==    by 0x698B9FE: open_components (mca_base_components_open.c:125)
==12372==    by 0x698B91B: mca_base_framework_components_open (mca_base_components_open.c:68)
==12372==    by 0x699A4B2: mca_base_framework_open (mca_base_framework.c:176)
==12372==    by 0x511281B: mca_pml_cm_component_open (pml_cm_component.c:112)
==12372==    by 0x698B9FE: open_components (mca_base_components_open.c:125)
==12372==    by 0x698B91B: mca_base_framework_components_open (mca_base_components_open.c:68)
==12372==    by 0x50E6AA9: mca_pml_base_open (pml_base_frame.c:183)
==12372==    by 0x699A49C: mca_base_framework_open (mca_base_framework.c:174)
==12372==

If I include unistd.h and use __environ instead the test program works. Looks like this happens because the values of env and __environ differ if setenv or putenv is called anywere in the program. You will have to drop the usage of the non-standard three-argument main.

hjelmn · 2015-12-17T19:46:09Z

Rebased off master.

hjelmn · 2015-12-21T17:34:51Z

:bot:retest:

hjelmn · 2015-12-22T15:20:36Z

:bot:retest:

hjelmn · 2016-01-04T18:38:23Z

:bot:retest:

hjelmn · 2016-02-23T18:03:23Z

:bot:retest:

hjelmn · 2016-02-24T22:33:51Z

:bot:retest:

hjelmn · 2016-03-08T22:21:26Z

@sjeaugey Thanks for running MTT on the branch. I was able to fix what I think was causing the problems.

Also, I making an attempt to put CUDA support in vader. Should have something ready this week.

bosilca · 2016-03-09T00:38:04Z

@hjelmn I would suggest you don't do it based on current SM support. We have a big PR coming with a significantly better approach (and also better performance). Would you be interested in attempting to add support in vader there ?

hjelmn · 2016-03-09T02:15:54Z

@bosilca Sure, I will wait. Once I get positive MTT results I will commit this PR. I will wait until your PR to look at smcuda.

hjelmn · 2016-03-09T16:36:36Z

@sjeaugey Looks like the updated PR wasn't run last night. Can you check what revision was run?

sjeaugey · 2016-03-09T16:50:11Z

@hjelmn I don't understand why it didn't run. Running it manually right now.

hjelmn · 2016-03-10T03:12:59Z

@sjeaugey Any results? If this is working I want to get it merged in as soon as possible.

hjelmn · 2016-03-10T14:21:37Z

@sjeaugey Looks like it ran last night and it looks clean. I will merge this later today.

hjelmn · 2016-03-10T15:46:08Z

:bot:retest

sjeaugey · 2016-03-10T17:01:40Z

@hjelmn Indeed it looks cleaner, except for one cuda test. However, I noticed yesterday that when running without any --mca btl argument (in fact when running with the default configuration) I got a segmentation fault on all the tests I tried. I don't know what we're using in this case (yalla?) but there seem to be an issue.

hjelmn · 2016-03-10T17:09:58Z

Might be worth adding a -mca pml_base_verbose 100 to see what pml is being selected.

hjelmn · 2016-03-10T21:17:58Z

@sjeaugey Found the bug that is causing the MTT failure. Will push after some testing.

hjelmn · 2016-03-10T21:21:23Z

@sjeaugey Ok, should be good for real now.

This commit rewrites both the mpool and rcache frameworks. Summary of changes: - Before this change a significant portion of the rcache functionality lived in mpool components. This meant that it was impossible to add a new memory pool to use with rdma networks (ugni, openib, etc) without duplicating the functionality of an existing mpool component. All the registration functionality has been removed from the mpool and placed in the rcache framework. - All registration cache mpools components (udreg, grdma, gpusm, rgpusm) have been changed to rcache components. rcaches are allocated and released in the same way mpool components were. - It is now valid to pass NULL as the resources argument when creating an rcache. At this time the gpusm and rgpusm components support this. All other rcache components require non-NULL resources. - A new mpool component has been added: hugepage. This component supports huge page allocations on linux. - Memory pools are now allocated using "hints". Each mpool component is queried with the hints and returns a priority. The current hints supported are NULL (uses posix_memalign/malloc), page_size=x (huge page mpool), and mpool=x. - The sm mpool has been moved to common/sm. This reflects that the sm mpool is specialized and not meant for any general allocations. This mpool may be moved back into the mpool framework if there is any objection. - The opal_free_list_init arguments have been updated. The unused0 argument is not used to pass in the registration cache module. The mpool registration flags are now rcache registration flags. - All components have been updated to make use of the new framework interfaces. As this commit makes significant changes to both the mpool and rcache frameworks both versions have been bumped to 3.0.0. Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

This means we need not check for jemalloc in the configure script for this component. Removing this. In some machines having the TLS option on can cause errors in opening this component. --disable-tls while configuring jemalloc. Please look for instructions for installing jemalloc as a static library linked directly into memkind in CONTRIBUTING file github.com/memkind/memkindw

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

sjeaugey · 2016-03-14T19:18:24Z

@hjelmn looks indeed good now !

sjeaugey · 2016-03-15T15:47:43Z

@nhjelmn Oh wait. The test count in MTT is not right. The IBM test were not run. Need to figure out why.

hjelmn · 2016-03-15T15:48:27Z

@sjeaugey Should be ok. Those tests don't have any CUDA specific so should be no different than what I see on my mlx4 systems.

mpool/rcache rewrite

hjelmn force-pushed the mpool_rewrite branch from 49e3584 to b8ed89d Compare November 10, 2015 18:43

hjelmn force-pushed the mpool_rewrite branch from b8ed89d to 43412e7 Compare November 11, 2015 15:34

hjelmn force-pushed the mpool_rewrite branch from 43412e7 to f114730 Compare November 12, 2015 05:23

hjelmn force-pushed the mpool_rewrite branch from d2bca24 to 2625f80 Compare November 24, 2015 16:11

hjelmn force-pushed the mpool_rewrite branch from 2625f80 to 8d40a02 Compare November 24, 2015 19:28

hjelmn force-pushed the mpool_rewrite branch from 8d40a02 to 18cfde5 Compare December 9, 2015 22:02

hjelmn force-pushed the mpool_rewrite branch 3 times, most recently from 65a7312 to 484f04d Compare December 9, 2015 22:27

hjelmn force-pushed the mpool_rewrite branch from 484f04d to 8efdc08 Compare December 17, 2015 19:44

hjelmn force-pushed the mpool_rewrite branch from 1d15e9a to 72b9947 Compare December 18, 2015 19:29

hjelmn force-pushed the mpool_rewrite branch from 72b9947 to c8f1a6c Compare January 6, 2016 19:32

hjelmn force-pushed the mpool_rewrite branch 2 times, most recently from 6614192 to fb833e8 Compare March 1, 2016 17:44

hjelmn force-pushed the mpool_rewrite branch from fb833e8 to 599344d Compare March 8, 2016 22:20

hjelmn force-pushed the mpool_rewrite branch from 599344d to 8d907db Compare March 10, 2016 21:21

hjelmn and others added 6 commits March 14, 2016 10:50

Adding memkind component to use MPI_Alloc_mem through memkind

3d98a1a

Update memkind mpool for new mpool interface

69d9266

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

rcache: add major/minor/release version macros

522c2f2

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

btl/usnic: update for mpool/rcache rewrite

eac0b11

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

hjelmn force-pushed the mpool_rewrite branch from 8d907db to eac0b11 Compare March 14, 2016 16:50

hjelmn added a commit that referenced this pull request Mar 15, 2016

Merge pull request #1118 from hjelmn/mpool_rewrite

ec97120

mpool/rcache rewrite

hjelmn merged commit ec97120 into open-mpi:master Mar 15, 2016

mpool/rcache rewrite #1118

mpool/rcache rewrite #1118

Uh oh!

Conversation

hjelmn commented Nov 10, 2015

Uh oh!

hjelmn commented Nov 10, 2015

Uh oh!

vvenkates27 commented Nov 11, 2015

Uh oh!

hjelmn commented Nov 11, 2015

Uh oh!

hjelmn commented Nov 23, 2015

Uh oh!

hjelmn commented Nov 23, 2015

Uh oh!

hjelmn commented Nov 24, 2015

Uh oh!

jsquyres commented Nov 30, 2015

Uh oh!

hppritcha commented Nov 30, 2015

Uh oh!

hjelmn commented Dec 9, 2015

Uh oh!

hjelmn commented Dec 9, 2015

Uh oh!

hjelmn commented Dec 9, 2015

Uh oh!

hjelmn commented Dec 11, 2015

Uh oh!

hjelmn commented Dec 15, 2015

Uh oh!

hjelmn commented Dec 17, 2015

Uh oh!

hjelmn commented Dec 17, 2015

Uh oh!

hjelmn commented Dec 21, 2015

Uh oh!

hjelmn commented Dec 22, 2015

Uh oh!

hjelmn commented Jan 4, 2016

Uh oh!

hjelmn commented Feb 23, 2016

Uh oh!

hjelmn commented Feb 24, 2016

Uh oh!

hjelmn commented Mar 8, 2016

Uh oh!

bosilca commented Mar 9, 2016

Uh oh!

hjelmn commented Mar 9, 2016

Uh oh!

hjelmn commented Mar 9, 2016

Uh oh!

sjeaugey commented Mar 9, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

sjeaugey commented Mar 10, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

hjelmn commented Mar 10, 2016

Uh oh!

sjeaugey commented Mar 14, 2016

Uh oh!

sjeaugey commented Mar 15, 2016

Uh oh!

hjelmn commented Mar 15, 2016

Uh oh!

Uh oh!