[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