Skip to content

Conversation

@xinzhao3
Copy link
Contributor

Signed-off-by: Xin Zhao [email protected]

@xinzhao3
Copy link
Contributor Author

cherrypick from #6290

ucp_rkey_pack(mca_spml_ucx.ucp_context, mkey->key.mem_h,
&(new_mkey->u.data), &len);

ucp_ep_rkey_unpack(ucx_ctx->ucp_peers[pe].ucp_conn,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works only for self, where are keys for other processes unpacked?


new_mkey->spml_context = new_ucx_mkey;

ucp_rkey_pack(mca_spml_ucx.ucp_context, mkey->key.mem_h,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to release rkey buf with ucp_rkey_buffer_release sometime

return module->get_mkey_slow(ctx, pe, va, rva);
} else {
uint32_t segno = memheap_find_segnum(va);
sshmem_mkey_t *new_mkey = (sshmem_mkey_t *)calloc(1, sizeof(*new_mkey));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need it?

}
} else {
assert(module->get_mkey_slow);
return module->get_mkey_slow(ctx, pe, va, rva);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably it will not request a new key from the remote, but will return the one cached for default context (if it's not a new mem region)

memheap_oob.mkeys = mkeys;
memheap_oob.segno = seg;
memheap_oob.mkeys_rcvd = 0;
memheap_oob.ctx = ctx;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be overwritten if several keys are requested in a row?

@gpaulsen
Copy link
Member

gpaulsen commented Mar 7, 2019

@xinzhao3 can you please address @brminich's questions/comments?

@gpaulsen gpaulsen added this to the v4.0.1 milestone Mar 11, 2019
@gpaulsen
Copy link
Member

@bwbarrett Why the Target:master label? This is currently based against v4.0.x (though I see there is no cherry-pick comment in the commit message. It looks like this is a cherry-pick of PR #6290).

@xinzhao3 Could you please add the "Cherry Picked from ... " message to the cherry-pick via the -x flag to git cherry-pick please?

@bwbarrett
Copy link
Member

@gpaulsen because I suck? :)

@hppritcha
Copy link
Member

@xinzhao3 we are planning to do another, and hopefully final rc on v4.0.1 on Monday. So please address his concerns by then, otherwise this PR will have to wait to 4.0.2.

@janjust
Copy link
Contributor

janjust commented Mar 20, 2019

@hppritcha: This PR is stale, should be replaced with #6509
@xinzhao3 please close this PR

@jladd-mlnx
Copy link
Member

This needs to be closed.

@xinzhao3 xinzhao3 closed this Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants