[openib-general] [PATCH 3/7] IB/ipoib - Use the new verbs DMA mapping functions

[Ralph Campbell]

On Sun, 2006-11-05 at 17:22 +0200, Michael S. Tsirkin wrote:
> Quoting r. Ralph Campbell <ralph.campbell at qlogic.com>:
> > diff -r f37bd0e41fec drivers/infiniband/ulp/ipoib/ipoib_ib.c
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c	Thu Oct 26 21:44:41 2006 +0700
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c	Thu Oct 26 12:37:09 2006 -0800
> > @@ -109,9 +109,8 @@ static int ipoib_ib_post_receive(struct 
> >  	ret = ib_post_recv(priv->qp, &param, &bad_wr);
> >  	if (unlikely(ret)) {
> >  		ipoib_warn(priv, "receive failed for buf %d (%d)\n", id, ret);
> > -		dma_unmap_single(priv->ca->dma_device,
> > -				 priv->rx_ring[id].mapping,
> > -				 IPOIB_BUF_SIZE, DMA_FROM_DEVICE);
> > +		ib_dma_unmap_single(priv->ca, priv->rx_ring[id].mapping,
> > +				    IPOIB_BUF_SIZE, DMA_FROM_DEVICE);
> >  		dev_kfree_skb_any(priv->rx_ring[id].skb);
> >  		priv->rx_ring[id].skb = NULL;
> >  	}
> 
> Hmm, since ib_dma_unmap_single calls a function through a pointer,
> this seems to introduce overhead on data path operations in ipoib.
> For apps like ipoib always working with low memory, I think it is important to avoid this
> overhead of extra indirect function calls at least on systems without IO MMU -
> where e.g. dma_unmap_single is empty.
> This probably means you need some of architecture-dependent code,
> but should be possible - look at how dma API is implemented for an example.
> And this applies to all ULPs on systems without high memory.

I did try asking the kernel folks if it would be acceptable to introduce
an architecture dependent way for device drivers to interpose on the
dma_*() functions but was rejected.

The open fabrics middle layer (IPoIB, RDS, SDP, SRP, etc.) is supposed
to be architecture neutral.  The only way I could think of to meet both
requirements is this sort of function indirection.
Note that for mtcha and ehca, there isn't a function call indirection.
The ib_dma_*() functions are inline and only test the ibdev->dma_ops
for NULL before calling the dma_*() inline functions.