On Nov 25, 2013, at 9:59 AM, Adrian Reber <adr...@lisas.de> wrote:
> * Send Non-blocking
> */
> int orte_rml_ftrm_send_nb(orte_process_name_t* peer,
> struct iovec* msg,
> int count,
> orte_rml_tag_t tag,
> - int flags,
> orte_rml_callback_fn_t cbfunc,
> void* cbdata)
> {
> int ret;
>
> opal_output_verbose(20, rml_ftrm_output_handle,
> - "orte_rml_ftrm: send_nb(%s, %d, %d, %d )",
> - ORTE_NAME_PRINT(peer), count, tag, flags);
> + "orte_rml_ftrm: send_nb(%s, %d, %d )",
> + ORTE_NAME_PRINT(peer), count, tag);
>
> if( NULL != orte_rml_ftrm_wrapped_module.send_nb ) {
> - if( ORTE_SUCCESS != (ret =
> orte_rml_ftrm_wrapped_module.send_nb(peer, msg, count, tag, flags, cbfunc,
> cbdata) ) ) {
> - return ret;
> - }
> - }
> -
> - return ORTE_SUCCESS;
> -}
> -
> -/*
> - * Send Buffer
> - */
> -int orte_rml_ftrm_send_buffer(orte_process_name_t* peer,
> - opal_buffer_t* buffer,
> - orte_rml_tag_t tag,
> - int flags)
> -{
> - int ret;
> -
> - opal_output_verbose(20, rml_ftrm_output_handle,
> - "orte_rml_ftrm: send_buffer(%s, %d, %d )",
> - ORTE_NAME_PRINT(peer), tag, flags);
> -
> - if( NULL != orte_rml_ftrm_wrapped_module.send_buffer ) {
> - if( ORTE_SUCCESS != (ret =
> orte_rml_ftrm_wrapped_module.send_buffer(peer, buffer, tag, flags) ) ) {
> + if( ORTE_SUCCESS != (ret =
> orte_rml_ftrm_wrapped_module.send_nb(peer, msg, count, tag, cbfunc, cbdata) )
> ) {
> return ret;
> }
> }
Similar to my reply about patch 3, I don't think this hunk is correct.
This routine accepts an iovec and sends it in a non-blocking fashion. I'll bet
that the caller frees the iovec upon return from the function (because it used
to be a blocking send, and freeing it immediately was acceptable).
But now the iovec may well still be in use when this function returns, so the
caller should *not* free/reuse the iovec until it knows that the send has
complete.
It may be more desirable to keep the blocking send function
orte_rml_ftrm_send_buffer() and emulate blocking by invoking send_nb under the
covers, but then not returning until the send callback has actually been
invoked.
Then the blocking semantics expected by the caller may well be acceptable/safe.
This loses some potential optimizations of asynchronicity, but it may be worth
it: a) performance in this part of the code isn't too critical, and b) blocking
semantics are usually simpler and easier to maintain, from the caller's
perspective.
This idea may also apply to what I said in reply to patch 3...? (i.e.,
preserve a blocking send by using the _nb variant under the covers, but not
returning until the nonblocking variant has actually completed the receive).
Since this is a fairly large change, I didn't look too closely throughout the
rest of this patch. I assume that there are a few other architectural cases
similar to this one.
--
Jeff Squyres
jsquy...@cisco.com
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/
