oshmem: sshmem: adds UCX allocator #2717
Conversation
| @@ -405,26 +406,32 @@ sshmem_mkey_t *mca_spml_ucx_register(void* addr, | |||
| return NULL; | |||
| } | |||
|
|
|||
| seg = memheap_find_segnum(addr); | |||
| segno = memheap_find_segnum(addr); | |||
There was a problem hiding this comment.
It is always valid because the register function is only called for memory segments that were already allocated and belong to the memheap.
|
@alex-mikheev please add signed-off-by |
|
@alex-mikheev should we deprecate the verbs allocator? Please PR into 1.10, 2.0.2, and 2.1. |
|
@alex-mikheev i don't see where memory is actually allocated with UCX? |
alex-mikheev
force-pushed
the
topic/sshmem_ucx
branch
from
bac1f16 to
1b4d25e
Compare
|
@jladd-mlnx not yet. ucx allocator can only be used by the ucx spml |
| @@ -112,9 +113,10 @@ typedef struct map_segment { | |||
| sshmem_mkey_t *mkeys; /* includes local segment bases in va_base */ | |||
| segment_flag_t flags; /* enable/disable flag */ | |||
| int seg_id; | |||
| char seg_name[OPAL_PATH_MAX]; | |||
There was a problem hiding this comment.
@alex-mikheev do you believe that seg_name is needless? I see that original code has unlink(ds_buf->seg_name) in shmem_ds_reset(). I do not remember if it is something useful but pay attention to avoid related issues.
There was a problem hiding this comment.
I see unlink() and memset two zero. However I do not see where seg_name is set to something and I do not see where it is used. (except in debug prints)
alex-mikheev
force-pushed
the
topic/sshmem_ucx
branch
from
1b4d25e to
daf110e
Compare
The commit removes unused code and interface function, moves common code to the base. Signed-off-by: Alex Mikheev <alexm@mellanox.com>
Signed-off-by: Alex Mikheev <alexm@mellanox.com>
Signed-off-by: Alex Mikheev <alexm@mellanox.com>
alex-mikheev
force-pushed
the
topic/sshmem_ucx
branch
from
daf110e to
c63137e
Compare
Signed-off-by: Alex Mikheev <alexm@mellanox.com>
|
bot:retest |
|
@alex-mikheev Please update v3.0.0 whitelist #3107 when you merge this. Check off ucx allocator button. Thanks. |
|
@yosefe Can we remove the DNM label? |
|
I think it needs to pass Mellanox Jenkins, too. Otherwise, all future PRs will fail Mellanox Jenkins. |
|
Looks like it's pulling in an older version of UCX that doesn't have the new flags defined. |
|
right, we need to make sure it passes with Mellanox jenkins. |
|
@alex-mikheev pls pick to v2.1 |
|
This is too late for 2.1
…On Thu, Mar 9, 2017 at 5:58 AM, Yossi ***@***.***> wrote:
@alex-mikheev <https://github.com/alex-mikheev> pls pick to v2.1
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2717 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHqGfGlZ7A-nwBkn5vrQu2aS1sdeZBIVks5rj9tRgaJpZM4LhzYV>
.
|
Also some code cleanup
@yosefe @jladd-mlnx @igor-ivanov