pml_ucx: add ompi datatype attribute to release ucp_datatype #5878
Conversation
Signed-off-by: Yossi Itigin <[email protected]>
|
adding @ca-taylor |
| ucp_worker_destroy(ompi_pml_ucx.ucp_worker); | ||
| ompi_pml_ucx.ucp_worker = NULL; | ||
| err: | ||
| return OMPI_ERROR; |
|
Will this make it into the 4.0 release? |
| PML_UCX_ASSERT(datatype->id < OMPI_DATATYPE_MAX_PREDEFINED); | ||
| ompi_pml_ucx.predefined_types[datatype->id] = ucp_datatype; | ||
| } else { | ||
| ret = ompi_attr_set_c(TYPE_ATTR, datatype, &datatype->d_keyhash, |
There was a problem hiding this comment.
Why do u need to set the ucp_datatype on the datatype->d_keyhash ? You have it stored in the pml_data anyways, right? The delete callback will be launched and you can get the ucp_type from pml_data. No, need to grow the hash, if i understand correctly.
There was a problem hiding this comment.
would the delete callback be called for an mpi datatype if i don't set my attribute?
according to the code of omi_attr_delete_impl - no:
https://github.com/open-mpi/ompi/blob/master/ompi/attribute/attribute.c#L1055
Signed-off-by: Yossi Itigin <[email protected]>
yosefe
force-pushed
the
topic/pml-ucx-fix-datatype-leak
branch
from
d3da92c to
40ac9e4
Compare
|
@AdamSimpson yes the plan is to backport to v4.0.x as well |
|
I see no trace of this fix in the 3.1.x or the 3.0.x branches, would probably be good to have it there too. |
|
@akesandgren, is it critical for you? Can you use OMPI 4.0x? |
|
We use everything from 1.6 to 3.1.4 at the moment, 4.x is in the pipeline (but at least ScaLAPACK need to get some fixes). So no, it's not critical, just highly preferable. |
Fixes openucx/ucx#2921
(checked that the leak is gone with IMB Allgatherv 512b, and passed mpi_test_suite)