> -----Original Message-----
> From: Roland Dreier [mailto:rol...@purestorage.com]
> Sent: Wednesday, March 21, 2012 9:44 PM
> To: Pandit, Parav
> Cc: linux-rdma@vger.kernel.org; net...@vger.kernel.org
> Subject: Re: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA
> adapter
>
> > +#define ocrdma_err(format, arg...) printk(KERN_ERR format, ##arg)
>
> I think you'd be better off using pr_err() rather than defining your own
> macro.
o.k. I'll change it.
>
>
> > +struct ocrdma_cq {
> > + struct ib_cq ibcq;
> > + struct ocrdma_dev *dev;
> > + struct ocrdma_cqe *va;
> > + u32 phase;
> > + u32 getp; /* pointer to pending wrs to
> > + * return to stack, wrap arounds
> > + * at max_hw_cqe
> > + */
> > + u32 max_hw_cqe;
> > + bool phase_change;
> > + bool armed, solicited;
> > + bool arm_needed;
> > +
> > + spinlock_t cq_lock ____cacheline_aligned; /* provide
> > + synchronization
> > + * to cq polling
> > + */
> > + /* syncronizes cq completion handler invoked from multiple
> > + context */
> > + spinlock_t comp_handler_lock ____cacheline_aligned;
>
> You have quite a few of these alignment directives in the middle of
> structures.
> Have you measured that leaving all these gaps gives a reall performance
> boost?
>
>From beginning its cacheline aligned. So didn't tested it without it, but yes
>this is considered. I'll be shifting other widely used elements before the
>lock so that hole is smaller.
> > + u16 id;
> > + u16 eqn;
> > +
> > + struct ocrdma_ucontext *ucontext;
> > + dma_addr_t pa;
> > + u32 len;
> > + atomic_t use_cnt;
> > +
> > + /* head of all qp's sq and rq for which cqes need to be
> > +flushed
> > + * by the software.
> > + */
> > + struct list_head sq_head, rq_head; };
>
>
> > +#define OCRDMA_GET_NUM_POSTED_SHIFT_VAL(qp) \
> > + (((qp->dev->nic_info.dev_family == OCRDMA_GEN2_FAMILY) && \
> > + (qp->id < 64)) ? 24 : 16)
>
> In general it's better to use inline functions when possible instead of
> macros,
> which are less type-safe and harder to read.
o.k. I'll change it.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
