[openib-general] [PATCH][1/7]ipoib performance patches -- remove ah_reap

[Shirley Ma]

Roland Dreier <rdreier at cisco.com> wrote on 05/24/2006 04:07:58 PM:
> To reiterate: freeing AHs is a rare,
> "slow path" operation that can be done asynchronously.  It is not a
> good tradeoff to do two atomic_t operations for every sent packet,
> just to avoid occasionally reaping AHs in process context.

I don't think two atomic operation is that expensive compare to 
reaping AHs in process context according to the test results and 
profiling data. Or we can use RCU instead.

>  > > But can you guarantee that the AH stays around until after the send
>  > > completes (which could be an arbitrarily long delay)?
> 
>  > I checked negih_add_path(), for unicast it is true always. See code 
below.
>  > 
>  > static void neigh_add_path(..)
>  > {
>  > ...
>  >         if (path->ah) {
>  >                 kref_get(&path->ah->ref);
>  >                 neigh->ah = path->ah;
>  >             ipoib_send(dev, skb, path->ah... 
>  > } 
> 
> Again, I don't understand how this is a response at all.  The AH
> cannot be freed until after the send operation is actually fully
> completed, which could be a long time after ib_post_send() returns.
> 
> If an AH is freed after ipoib_send() returns but before the send is
> executed, then the HCA may use stale data, which could lead to a send
> error.
> 
> To summarize: the patch is broken (leads to incorrect lifetimes for
> AHs), and in any case makes the send fast path slower.
> 
>  - R.

That's a value point. This problem will be addressed in next tx_ring 
removal 
patch. The kref_put was called in ipoib_ib_handle_send_wc().

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone(Fax): (503) 578-7638
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20060524/5f9707d6/attachment.html>