[openib-general] [PATCH][5/18] InfiniBand/mthca: add needed rmb() in event queue poll
Michael S. Tsirkin
mst at mellanox.co.il
Thu Jan 13 01:45:32 PST 2005
Hello!
Quoting r. Roland Dreier (roland at topspin.com) "[openib-general] [PATCH][5/18] InfiniBand/mthca: add needed rmb() in event queue poll":
> Add an rmb() between checking the ownership bit of an event queue
> entry and reading the contents of the EQE. Without this barrier, the
> CPU could read stale contents of the EQE before HW writes the EQE but
> have the read of the ownership bit reordered until after HW finishes
> writing, which leads to the driver processing an incorrect event. This
> was actually observed to happen when multiple completion queues are in
> heavy use on an IBM JS20 PowerPC 970 system.
>
> Also explain the existing rmb() in completion queue poll (there for
> the same reason) and slightly improve debugging output.
>
> Signed-off-by: Roland Dreier <roland at topspin.com>
>
> --- linux/drivers/infiniband/hw/mthca/mthca_cq.c (revision 1437)
> +++ linux/drivers/infiniband/hw/mthca/mthca_cq.c (revision 1439)
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004 Topspin Communications. All rights reserved.
> + * Copyright (c) 2004, 2005 Topspin Communications. All rights reserved.
> *
> * This software is available to you under a choice of one of two
> * licenses. You may choose to be licensed under the terms of the GNU
> @@ -391,6 +391,10 @@
> if (!next_cqe_sw(cq))
> return -EAGAIN;
>
> + /*
> + * Make sure we read CQ entry contents after we've checked the
> + * ownership bit.
> + */
> rmb();
>
> cqe = get_cqe(cq, cq->cons_index);
> @@ -768,7 +772,8 @@
> u32 *ctx = MAILBOX_ALIGN(mailbox);
> int j;
>
> - printk(KERN_ERR "context for CQN %x\n", cq->cqn);
> + printk(KERN_ERR "context for CQN %x (cons index %x, next sw %d)\n",
> + cq->cqn, cq->cons_index, next_cqe_sw(cq));
> for (j = 0; j < 16; ++j)
> printk(KERN_ERR "[%2x] %08x\n", j * 4, be32_to_cpu(ctx[j]));
> }
> --- linux/drivers/infiniband/hw/mthca/mthca_eq.c (revision 1437)
> +++ linux/drivers/infiniband/hw/mthca/mthca_eq.c (revision 1439)
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2004 Topspin Communications. All rights reserved.
> + * Copyright (c) 2004, 2005 Topspin Communications. All rights reserved.
> *
> * This software is available to you under a choice of one of two
> * licenses. You may choose to be licensed under the terms of the GNU
> @@ -240,6 +240,12 @@
> int set_ci = 0;
> eqe = get_eqe(eq, eq->cons_index);
>
> + /*
> + * Make sure we read EQ entry contents after we've
> + * checked the ownership bit.
> + */
> + rmb();
> +
> switch (eqe->type) {
> case MTHCA_EVENT_TYPE_COMP:
> disarm_cqn = be32_to_cpu(eqe->event.comp.cqn) & 0xffffff;
Since we are using the eqe here, it seems that read_barrier_depends
shall be sufficient (as well as in the cq case)?
However, I see that read_barrier_depends is a nop on ppc, and the
comment indicates that problems were seen on ppc 970.
What gives? do I misunderstand what a dependency is?
MST
More information about the general
mailing list