On Thu, Dec 28, 2017 at 10:18:02PM +0800, Yixian Liu wrote:
> This patch adds more detailed comments when we call the
> memory barrier function, such as rmb, wmb and mb. Three
> mb() callers are deleted since they are unnecessary.
>
> v3:
> 1. Subsitute rmb() with dma_rmb() according to Jason's comment,
> since using dma_rmb() is enough.
This is such a big improvement, thanks
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 6100ace..47ef90b 100644
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -3963,7 +3963,10 @@ static void set_eq_cons_index_v1(struct hns_roce_eq *eq, int req_not)
> {
> roce_raw_write((eq->cons_index & HNS_ROCE_V1_CONS_IDX_M) |
> (req_not << eq->log_entries), eq->doorbell);
> - /* Memory barrier */
> +
> + /* Make sure we update the consumer index of EQ before
> + * accessing it later
> + */
> mb();
> }
This (and the other one below like it) is the only one that puzzles
me..
What are you trying to accomplish here? It looks like cons_index is
the tail pointer for a ring that the device DMA's into? And the
dma_rmb seems properly placed, so no barrier should be needed for DMA
when updating the tail pointer.
Is the barrier attempting to protect eq->cons_index in system memory?
If yes, shouldn't it be an atomic and/or locked?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
