Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] [PATCH 2/4] Trying to get the C/R code to compile again. (send_*_nb)
From: Jeff Squyres (jsquyres) (jsquyres_at_[hidden])
Date: 2013-12-04 10:54:21


On Nov 25, 2013, at 9:59 AM, Adrian Reber <adrian_at_[hidden]> 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
jsquyres_at_[hidden]
For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/