SPML/UCX: removed direct dependency to SPML UCX#8567
Conversation
hoopoepg
force-pushed
the
topic/ucx-atomic-dependencies
branch
from
b0af83e to
5c76d8a
Compare
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
| .ucp_peers = NULL, | ||
| .options = 0 | ||
| .options = 0, | ||
| .spml_ucx = &mca_spml_ucx |
There was a problem hiding this comment.
maybe just move synchronized_quiet to this struct? instead of moving &mca_spml_ucx
There was a problem hiding this comment.
in this case we have to additionally handle initialization of synchronized_quiet and update this value in global object - it makes fix more complicated
There was a problem hiding this comment.
i think it will worth it: better to keep "mca_spml_ucx" private, and mca_spml_ucx_ctx_t "shared"
and not point from "shared" to "private"
There was a problem hiding this comment.
ok, then may be pointer to synchronized_quiet?
There was a problem hiding this comment.
i think better just move this bool field. no need to keep it in 2 places.
|
Is this master / v5.0 only? or intended for bringing back to v4.0.x/v4.1.x? |
|
as I can see 4.x affected too... |
oshmem/mca/spml/ucx/spml_ucx.c
Outdated
| rc = mca_spml_ucx_ctx_create_common(SHMEM_CTX_PRIVATE, &mca_spml_ucx.aux_ctx); | ||
| if (OSHMEM_SUCCESS == rc) { | ||
| mca_spml_ucx.aux_ctx->spml_ucx = &mca_spml_ucx; | ||
| mca_spml_ucx.aux_ctx->synchronized_quiet = mca_spml_ucx_ctx_default.synchronized_quiet; |
There was a problem hiding this comment.
should move it inside mca_spml_ucx_ctx_create_common
There was a problem hiding this comment.
will not work: there is array of pre-allocated objects which created without mca_spml_ucx_ctx_create_common call
There was a problem hiding this comment.
but mca_spml_ucx_ctx_create_common already sets other fields of *mca_spml_ucx.aux_ctx
- added synchronise_quiet parameter to local context object Signed-off-by: Sergey Oblomov <sergeyo@nvidia.com>
hoopoepg
force-pushed
the
topic/ucx-atomic-dependencies
branch
from
5f6f176 to
01d7164
Compare
|
Seems to work for me. 👍 |
|
@hoopoepg could you please merge, and then PR to release branches? Thank you. |
4 participants
local context datatype to eliminate linker
dependency
fixes #8542