[ofa-general] Re: [PATCH draft, untested] ehca srq emulation (for IPoIB CM)

[Pradeep Satyanarayana]

Roland Dreier wrote:
>  > In the absence of any further discussions about the IPoIB CM without SRQ
>  > patches, I will incorporate Sean Hefty's comments and plan to resubmit
>  > the patches, unless I hear something soon.
> 
> Sorry for not devoting enough time to this, but something always seems
> to come up, and I really want to be able to focus a concentrated chunk
> of time on this, and I never seem to be able to.  Anyway, I would
> prefer to find a solution that everyone can agree on, without me
> having to rule by decree.
> 
> I think updating the patch is a good idea.  Although I didn't get a
> chance to review it carefully there were a number of obvious messy
> parts that should be cleaned up.
> 
> I am beginning to think that your basic approach is probably right,
> but I also still think it should be possible to handle both SRQ and
> non-SRQ without any overhead on the fast path.  I don't understand the
> "maintainability" argument against doing this.  Can you expand on your
> position a little?
> 

I will try to illustrate with an example:

One of the ways to do this is to completely split SRQ and non-SRQ
processing starting in ipoib_poll(). This would eliminate most of
the if (srq) kind of branches. However, there would be a lot of code
duplication. If a bug is discovered in one path, then one needs to
fix that in the other path too.

One way to mitigate this situation is to alter the current SRQ code
to use common code (between SRQ and non-SRQ). However, one might not 
want to factor off a few lines of common code into a new function. There
may be several such occurrences of this resulting in code bloat.

If you look back, several weeks ago ipoib_drain_cq() did not exist. This
is another function that calls ipoib_cm_handle_rx_wc(). We would need
to alter this function too to accommodate SRQ and non-SRQ split. In
effect, we have propagated the SRQ and non-SRQ code to functions
outside ipoiob_cm.c. In the future, if IPoIB CM would support UC mode
this might mean additional functions handling the split.

On the other hand, in V6 (and previous versions) of the patch
ipoib_cm_handle_rx_wc() handles the SRQ and non-SRQ paths. Both SRQ and
non-SRQ functionality is contained within ipoib_cm.c. What we now have
is probably one extra branch in the packet handling path than the
minimum (desired) with a lot of common code.

Pradeep