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
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
- application/pkcs7-signature attachment: smime.p7s
|
![[Flag]](../../../../../community/mirrors/flag-us.gif){kind=small}