-
Notifications
You must be signed in to change notification settings - Fork 932
Topic/oshmem mem prefetch v2.x #2872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Topic/oshmem mem prefetch v2.x #2872
Conversation
The hook is called from memheap when memory range is going to be allocated by smalloc(), realloc() and others. ucx spml uses this hook to call ucp_mem_advise in order to speedup non blocking memory mapping. Signed-off-by: Alex Mikheev <alexm@mellanox.com> (cherry picked from commit 986ca00)
Signed-off-by: Alex Mikheev <alexm@mellanox.com> (cherry picked from commit 9da9e62)
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
| &mca_spml_ucx.ucp_peers[i].ucp_conn); | ||
| if (UCS_OK != err) { | ||
| SPML_ERROR("ucp_ep_create failed!!!\n"); | ||
| SPML_ERROR("ucp_ep_create failed: %s\n", ucs_status_string(err)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can remote the \n
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
| err = ucp_worker_flush(mca_spml_ucx.ucp_worker); | ||
| if (UCS_OK != err) { | ||
| SPML_ERROR("fence failed"); | ||
| SPML_ERROR("fence failed: %s", ucs_status_string(err)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
| err = ucp_worker_flush(mca_spml_ucx.ucp_worker); | ||
| if (UCS_OK != err) { | ||
| SPML_ERROR("fence failed"); | ||
| SPML_ERROR("fence failed: %s", ucs_status_string(err)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
|
|
||
| *p_buff = (void*) addr; | ||
| /* no barrier because it is not required by spec! */ | ||
| MCA_SPML_CALL(memuse_hook(addr, 1<<order)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1LL<<order
2nd param is size_t or cast order to (size_t)
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
|
|
||
| void mca_spml_ucx_memuse_hook(void *addr, size_t length) | ||
| { | ||
| int my_pe = oshmem_my_proc_id(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plz split into var and assignment
|
@yosefe @miked-mellanox can you rollback the merge of original comits to master ? I would rather fix it there and than cherry pick again |
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
|
|
||
| status = ucp_mem_advise(mca_spml_ucx.ucp_context, ucx_mkey->mem_h, ¶ms); | ||
| if (UCS_OK != status) { | ||
| SPML_ERROR("ucp_mem_advise failed addr %p len %llu", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe warn? as it is not destructive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no option for warning. It is either verbose or error.
|
@alex-mikheev i think it would be easier to submit a fix commit both to master and to this PR |
Signed-off-by: Alex Mikheev <alexm@mellanox.com>
|
@alex-mikheev - please PR it into v2.x and v2.0 |
|
bot:lanl:retest |
|
@hppritcha Please merge this ASAP. The LANL failure looks unrelated to this commit. |
|
bot:lanl:retest |
@yosefe @miked-mellanox @jladd-mlnx
please take a look