[ofa-general] IPOIB CM (NOSRQ) patch for review

Roland Dreier rdreier at cisco.com
Mon Mar 19 12:16:04 PDT 2007


 > I dug through the spec and found that ib_query_device() tells one if the 
 > HCA supports SRQ or not. Is that what you had in mind?

Yes, something like that.  To see why the test has to be at runtime
rather than a compile time decision, just think about a distribution
kernel -- how can they ship IPoIB CM that works for adapters that
don't support SRQ if that breaks disables CM for adapters that do
support SRQ?

 > > Not to mention the fact that basically mixing together two different
 > > implementations with a liberal sprinkling of #ifdef IPOIB_CM_NOSRQ
 > > makes the code basically unreadable and unmaintainable.
 > 
 > One way to alleviate this problem would be to duplicate mainly the 
 > receive side functions and name them something like xyz_nosrq(). 
 > However, there will still be many instances of
 > 
 > if(SRQ)
 >         xyz();
 > else
 >         xyz_nosrq();
 > 
 > Is that a better solution than the #ifdef IPOIB_CM_NOSRQ? On the
 > other hand, this will add duplicate code and may pose some
 > maintainability issues in the future. I would like to understand as
 > to which one is the preferred approach.

Well, anything is better than the #ifdef stuff, since in addition to
being out of the question for the reasons I outlined above, it also
has the problem that testing changes requires two builds to see if
anything broke, etc.

 > > This seems crazy -- you do a linear search through a list of QPs
 > > (which potentially has 100s of entries) for every receive completion!
 > > Just the spinlock alone is something we would want to avoid in the hot
 > > performance path.
 > 
 > I envisaged the NOSRQ case for small clusters only. Othewise, this
 > may be a memory hog and affect (other) application performance.

There has to be a better way.  You have 64 bits of work request ID to
work with, surely you can avoid a linear search and a spinlock to do
this lookup.

 - R.



More information about the general mailing list