[openib-general] Re: [PATCH] repost: IPoIB queue size tune patch

[Michael S. Tsirkin]

Quoting r. Shirley Ma <xma at us.ibm.com>:
> Subject: Re: [PATCH] repost: IPoIB queue size tune patch
> 
> 
> Hello Roland,
> 
> Thanks.
> 
> Roland Dreier <rdreier at cisco.com> wrote on 04/03/2006 10:50:37 PM:
> 
> > Sorry I hadn't gotten a chance to read this over until now...
> >
> >  > -       IPOIB_RX_RING_SIZE        = 128,
> >  > -       IPOIB_TX_RING_SIZE        = 64,
> >  > +       IPOIB_SENDQ_SIZE          = 64,
> >  > +       IPOIB_RECVQ_SIZE          = 128,
> >
> > Can you explain again why it's a good idea to rename these?  Is the
> > name "sendq_size" really clearer than "tx_ring_size," especially in
> > the context of a network driver?
> 
> tx_ring and rx_ring are not good names for IPoIB. tx_ring in IPoIB is a place
> to save pointers and rx_ring a place to allocate skb buffs.

Actually names make sense to me - they are cyclic buffers, hence ring.

> I am working on
> a patch to remove tx_ring and replace rx_ring with a list, which would
> reduce some tx_ring overhead.

What overhead?  Please don't, lists are bad for cache, require locks ...
Circular buffers are much better, I don't have the time now but I think we can
even make event handler completely lockless with them.

> >
> >  > +       int     sendq_size;
> >  > +       int     recvq_size;
> >
> > Why does every device need a private copy of the ring sizes?  It seems
> > better to just use ipoib_sendq_size and ipoib_recvq_size directly --
> > round them up in the module init function, and I guess mark them
> > __read_mostly.
> >
> 
> Michael suggested to save them in the priv before. I did some test and didn't
> see the difference. I wil move them out of priv.

The point was to make the parameter writeable in sysfs.
If you don't do this there's no point.

-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies