There is something weird with this change, and the patch reflect it. The new argument "order" come from the PML level and might be MCA_BTL_NO_ORDER (which is kind of global) or BTL_OPENIB_LP_QP or BTL_OPENIB_HP_QP (which are definitively Open IB related). Do you really intend to let the PML knows about Open IB internal constants ? If it's the case (which seems to be true from the following snippet
    if(MCA_BTL_NO_ORDER == order) {
        frag->base.order = BTL_OPENIB_LP_QP;
    } else {
        frag->base.order = order;
    }
) I expect you to revise the patch in order to propose a generic solution or I'll trigger a vote against the patch. I vote to be backed out of the trunk as it export way to much knowledge from the Open IB BTL into the PML layer.

  george.

PS: With Gleb changes the problem is the same. The following snippet reflect exactly the same behavior as the original patch.

frag->base.order = order;
assert(frag->base.order != BTL_OPENIB_HP_QP);

On Jun 7, 2007, at 9:49 AM, Gleb Natapov wrote:

Hi Galen,

On Sun, May 27, 2007 at 10:19:09AM -0600, Galen Shipman wrote:

With current code this is not the case. Order tag is set during a
fragment
allocation. It seems wrong according to your description. Attached
patch fixes
this. If no specific ordering tag is provided to allocation
function order of
the fragment is set to be MCA_BTL_NO_ORDER. After call to send/put/
get order
is set to whatever QP was used for communication. If order is set
before send call
it is used to choose QP.


I do set the order tag during allocation/prepare, but the defined
semantics are that the tag is only valid after send/put/get. We can
set them up any where we wish in the BTL, the PML however cannot rely
on anything until after the send/put/get call. So really this is an
issue of semantics versus implementation. The implementation I
believe does conform to the semantics as the upper layer (PML)
doesn't use the tag value until after a call to send/put/get.

I will look over the patch however, might make more sense to delay
setting the value until the actual send/put/get call.

Have you had a chance to look over the patch?

--
                        Gleb.
_______________________________________________
devel mailing list
de...@open-mpi.org
http://www.open-mpi.org/mailman/listinfo.cgi/devel

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to