Michael,
just is questions about the code...maybe roland can answer.
On Wed, Jan 26, 2005 at 04:52:18PM +0200, Michael S. Tsirkin wrote:
> Index: hw/mthca/mthca_cq.c
> ===================================================================
> --- hw/mthca/mthca_cq.c (revision 1653)
> +++ hw/mthca/mthca_cq.c (working copy)
> @@ -147,20 +147,21 @@ static inline struct mthca_cqe *get_cqe(
> + (entry * MTHCA_CQ_ENTRY_SIZE) % PAGE_SIZE;
> }
>
> -static inline int cqe_sw(struct mthca_cq *cq, int i)
> +static inline struct mthca_cqe *cqe_sw(struct mthca_cq *cq, int i)
> {
> - return !(MTHCA_CQ_ENTRY_OWNER_HW &
> - get_cqe(cq, i)->owner);
> + struct mthca_cqe *cqe;
> + cqe = get_cqe(cq, i);
this could be one line:
struct mthca_cqe *cqe = get_cqe(cq, i);
> @@ -775,7 +784,7 @@ void mthca_free_cq(struct mthca_dev *dev
> int j;
>
> printk(KERN_ERR "context for CQN %x (cons index %x, next sw
> %d)\n",
> - cq->cqn, cq->cons_index, next_cqe_sw(cq));
> + cq->cqn, cq->cons_index, next_cqe_sw(cq)?1:0);
> for (j = 0; j < 16; ++j)
> printk(KERN_ERR "[%2x] %08x\n", j * 4,
> be32_to_cpu(ctx[j]));
> }
This code chunk is "dead" code. It starts with "if (0)".
Can it just be deleted?
In mthca_poll_one():
if ((cqe->opcode & MTHCA_ERROR_CQE_OPCODE_MASK) ==
MTHCA_ERROR_CQE_OPCODE_MASK) {
is_error = 1;
is_send = cqe->opcode & 1;
} else
is_send = cqe->is_send & 0x80;
Could be better written as:
is_error = (cqe->opcode & MTHCA_ERROR_CQE_OPCODE_MASK) ==
MTHCA_ERROR_CQE_OPCODE_MASK);
is_send = is_error ? (cqe->opcode & 1) : (cqe->is_send & 0x80);
BTW, I dislike use of "is_send" to branch the code path several times
later instead of just having two different code paths that call the same
in-line functions. Interested in a patch to change this?
thanks,
grant
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
