OSC/UCX: fixed zero-size window processing #5879
Conversation
ompi/mca/osc/ucx/osc_ucx_comm.c
Outdated
| } | ||
| } | ||
|
|
||
| if (!((module->win_info_array[target]).rkey_init) && (target_count > 0)) { |
There was a problem hiding this comment.
maybe add this as macro/common function?
also looks like two pairs of redundant () around !((module->win_info_array[target]).rkey_init)
ompi/mca/osc/ucx/osc_ucx_comm.c
Outdated
| } | ||
|
|
||
| if (!((module->win_info_array[target]).rkey_init) && (target_count > 0)) { | ||
| OSC_UCX_VERBOSE(1, "target window has no rkey or has zero size"); |
There was a problem hiding this comment.
"window with non-zero length does not have an rkey"
| memcpy((void *)((char *)my_info + 2 * sizeof(uint64_t)), rkey_buffer, rkey_buffer_size); | ||
| memcpy((void *)((char *)my_info + 2 * sizeof(uint64_t) + rkey_buffer_size), | ||
| state_rkey_buffer, state_rkey_buffer_size); | ||
| memcpy_off(my_info, &state_base, sizeof(uint64_t), info_offset); |
There was a problem hiding this comment.
maybe add the window size as another uint64_t in the beginning (in addition to state_base and rkey_buffer_size)?
There was a problem hiding this comment.
didn't get your comment, size is added on next line (line 544 of updated file)
There was a problem hiding this comment.
i mean, in terms of packing, do '3 * sizeof(uint64_t)' instead of 2 * sizeof(uint64_t)'
There was a problem hiding this comment.
hmmmm, size is size_t type, not uint64_t, merge into 3 * sizeof(uint64_t) ?
|
Ok, this needs to v3.1.x and v4.0.x today. Please squish all commits into a single commit. Not supporting 0-sized windows is a critical bug. |
|
@yosefe ok to approve or more comments? |
|
@yosefe I will squash commits after tests are completed, will ping you when it ready to commit |
hoopoepg
force-pushed
the
topic/fixed-zero-size-window
branch
from
434a201 to
b91164f
Compare
- added processing of zero-size MPI window Signed-off-by: Sergey Oblomov <[email protected]>
hoopoepg
force-pushed
the
topic/fixed-zero-size-window
branch
from
b91164f to
ae6f819
Compare
|
bot:retest |
|
@yosefe ok to merge? |
|
@hoopoepg need for v4.0.x |
3 participants
fixes #5873
Signed-off-by: Sergey Oblomov [email protected]