[ofw][patch][ND provider] Improving latency of ms-mpi
Leonid Keller
leonid at mellanox.co.il
Sun Aug 16 05:45:11 PDT 2009
Committed to trunk and branch - 2352-3.
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Leonid Keller
> Sent: Friday, August 14, 2009 11:37 AM
> To: Fab Tillier; Tzachi Dar; Sean Hefty
> Cc: ofw at lists.openfabrics.org
> Subject: RE: [ofw][patch][ND provider] Improving latency of ms-mpi
>
>
> OK, I return pMaxInlineData to be only an out parameter.
> I've also added some little improvement according to Fab comments.
>
> I think, this patch should go also to the release branch.
> Agree ?
>
> Index: ulp/nd/user/NdEndpoint.cpp
> ===================================================================
> --- ulp/nd/user/NdEndpoint.cpp (revision 2351)
> +++ ulp/nd/user/NdEndpoint.cpp (working copy)
> @@ -98,7 +98,7 @@
> __in SIZE_T nOutboundSge,
> __in SIZE_T InboundReadLimit,
> __in SIZE_T OutboundReadLimit,
> - __in_opt __out_opt SIZE_T* pMaxInlineData
> + __out_opt SIZE_T* pMaxInlineData
> )
> {
> ND_ENTER( ND_DBG_NDI );
> @@ -124,11 +124,7 @@
> m_pParent->m_Ifc.user_verbs.post_recv != NULL /*||
> m_pParent->m_Ifc.user_verbs.bind_mw != NULL*/ );
>
> - UINT32 InlineSize;
> - if ( pMaxInlineData )
> - InlineSize = (UINT32)*pMaxInlineData;
> - else
> - InlineSize = g_nd_max_inline_size;
> + m_MaxInlineSize = g_nd_max_inline_size;
>
> HRESULT hr = CreateQp(
> pInboundCq,
> @@ -139,7 +135,7 @@
> nOutboundSge,
> InboundReadLimit,
> OutboundReadLimit,
> - InlineSize );
> + m_MaxInlineSize );
>
> if( FAILED( hr ) )
> return hr;
> @@ -151,12 +147,11 @@
> return hr;
> }
> else
> - InlineSize = (UINT32)qp_attr.sq_max_inline;
> + m_MaxInlineSize = (UINT32)qp_attr.sq_max_inline;
>
>
> m_Ird = (UINT8)InboundReadLimit;
> m_Ord = (UINT8)OutboundReadLimit;
> - m_MaxInlineSize = InlineSize;
>
> // Move the QP to the INIT state so users can post receives.
> hr = ModifyQp( IB_QPS_INIT );
> @@ -164,7 +159,7 @@
> DestroyQp();
>
> if( SUCCEEDED( hr ) && pMaxInlineData != NULL )
> - *pMaxInlineData = InlineSize;
> + *pMaxInlineData = m_MaxInlineSize;
>
> return hr;
> }
> Index: ulp/nd/user/NdEndpoint.h
> ===================================================================
> --- ulp/nd/user/NdEndpoint.h (revision 2351)
> +++ ulp/nd/user/NdEndpoint.h (working copy)
> @@ -67,7 +67,7 @@
> __in SIZE_T nOutboundSge,
> __in SIZE_T InboundReadLimit,
> __in SIZE_T OutboundReadLimit,
> - __in_opt __out_opt SIZE_T* pMaxInlineData
> + __out_opt SIZE_T* pMaxInlineData
> );
>
> public:
>
> > -----Original Message-----
> > From: Fab Tillier [mailto:ftillier at microsoft.com]
> > Sent: Wednesday, August 12, 2009 7:59 PM
> > To: Tzachi Dar; Sean Hefty; Leonid Keller
> > Cc: ofw at lists.openfabrics.org
> > Subject: RE: [ofw][patch][ND provider] Improving latency of ms-mpi
> >
> > I think specifying a default of 160 (or whatever is appropriate for
> > the hardware), that can be overridden via an environment
> variable is
> > the right thing to do. Changing the parameter to be in/out is the
> > wrong thing to do at this point in time. Changing the API
> contract is
> > only going to confuse application developers, and in the
> longer term
> > will just result in support issues.
> >
> > The max inline parameter is an out parameter. Until there's a new
> > interface that defines it otherwise, it should remain an out
> > parameter.
> >
> > -Fab
> >
> > > -----Original Message-----
> > > From: Tzachi Dar [mailto:tzachid at mellanox.co.il]
> > > Sent: Wednesday, August 12, 2009 4:40 AM
> > > To: Sean Hefty; Leonid Keller; Fab Tillier
> > > Cc: ofw at lists.openfabrics.org
> > > Subject: RE: [ofw][patch][ND provider] Improving latency of ms-mpi
> > >
> > > I guess that we all have good points here, but I believe we
> > should be
> > > moving forward:
> > >
> > > Since an important goal of this project is reaching low
> latency, I
> > > don't see how we can not do inline send at all.
> > >
> > > I agree that changing the api might break existing
> applications but:
> > > 1) I don't think there is really a big number of
> > applications
> > > currently. 2) The "break" actually means that
> inline will be
> > > used on a
> > > place where it is not supposed to. This might mean "sub-optimal"
> > > performance, but I believe that all applications will
> > continue to work.
> > > (We need to make sure that if the number used is too big,
> > we will use
> > > a good default (maybe 0).
> > > 3) Giving applications control over the inline size
> > (that is
> > > using the parameter as in/out) will be the best technical
> solution
> > > going forward.
> > >
> > > Our suggestion is this: add a new environment variable. If that
> > > variable doesn't exist the parameter is out only. If it
> is set, the
> > > parameter is in/out. This will promise that older
> > applications still work as well.
> > > People who write according to the new api will set this
> environment
> > > variable. Fab, can you make sure that the new nd version
> will allow
> > > this parameter to be in/out?
> > >
> > > Controlling inline or not will still be done using an environment
> > > variable (as Leonid sent).
> > >
> > > Does this makes sense?
> > >
> > > Thanks
> > > Tzachi
> > >
> > >
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Sean Hefty [mailto:sean.hefty at intel.com]
> > >> Sent: Wednesday, August 12, 2009 2:03 AM
> > >> To: Leonid Keller; Fab Tillier; Tzachi Dar
> > >> Cc: ofw at lists.openfabrics.org
> > >> Subject: RE: [ofw][patch][ND provider] Improving latency
> of ms-mpi
> > >>
> > >> My point was that NetworkDirect is a published API whose
> > definition
> > >> is owned entirely by Microsoft. All implementations MUST
> > adhere to
> > >> the published ND API specification, or they are not
> > compliant. It's
> > >> not about legality, who maintains a specific ND
> implementation, or
> > >> whether a specific change is considered an improvement
> over what's
> > >> there.
> > >>
> > >> We are free to change IBAL, WinVerbs, or other APIs
> because we own
> > >> them. The only cost of doing so is breaking existing
> > applications.
> > >> But for ND, our choice is to be compliant or not.
> > >>
> > >> The way I've gone about requesting changes to ND is to
> > send comments
> > >> to MS using the links at the bottom of the ND documentation. I
> > >> expect that these messages get routed directly to Fab, who
> > rolls his
> > >> eyes before hitting the delete key.
> > >> :)
> > >>
> > >>
> > >>> This change adds a new facility. Tzachi and me have brought the
> > >>> reasons. So it's good from engineer point view. I don't
> > know whether
> > >>> we may do it from legal one. But we used to change Ibal
> > API and ND
> > >>> sits in the same Open Source Tree with IBAL. Who is he
> > maintainer of
> > >>> ND provider code ? I thought it's Fab. Fab, do you
> > approve this API
> > >>> change ?
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Sean Hefty [mailto:sean.hefty at intel.com]
> > >>>> Sent: Monday, August 10, 2009 9:05 PM
> > >>>> To: Leonid Keller; Fab Tillier; Tzachi Dar
> > >>>> Cc: ofw at lists.openfabrics.org
> > >>>> Subject: RE: [ofw][patch][ND provider] Improving latency
> > of ms-mpi
> > >>>>
> > >>>>> This latter parameter should be IN OUT, because the
> > driver takes
> > >>>>> its value as a hint. It really re-calculates it, trying to
> > >>>>> maximize in the limits of WQE size.
> > >>>>
> > >>>> Isn't this a MS defined API? I don't believe we can
> change the
> > >>>> NDEndpoint APIs at all.
> > >>>>
> > >>>> - Sean
> > >>>>
> > >>>>
> > >>
> > >>
> >
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
>
More information about the ofw
mailing list