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? > + 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. I wonder if it's also worth having masks to avoid subtracting 1 every time the driver does something like > + tx_req = &priv->tx_ring[priv->tx_head & (priv->sendq_size - 1)]; > +#define IPOIB_MAX_QUEUE_SIZE 4096 /* max is 4k */ > +#define IPOIB_MIN_QUEUE_SIZE 64 /* min is 64 */ Where do these limits come from? Why shouldn't someone be able to use a bigger or smaller ring? > printk(KERN_WARNING "%s: failed to allocate RX ring (%d > entries)\n", > - ca->name, IPOIB_RX_RING_SIZE); > + ca->name, priv->sendq_size); Looks like this should be recvq_size, not sendq_size. > + printk(KERN_INFO "%s: RX_RING_SIZE is set to %d entries\n", > + ca->name, priv->recvq_size); Seems kind of chatty -- I think anyone who cared could just look at the module parameter in sysfs. - R. _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
