netdev - Re: [PATCH 1/9] ocrdma: Driver for Emulex OneConnect RDMA adapter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Mar 2012 09:14:12 -0700
From:	Roland Dreier <roland@...estorage.com>
To:	parav.pandit@...lex.com
Cc:	linux-rdma@...r.kernel.org, netdev@...r.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.


> +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?

> +       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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html