Quoting r. Grant Grundler <[EMAIL PROTECTED]>:
> Subject: Re: [openib-general] [PATCH] (fixed) cqe lookup speedup
>
> Michael,
> just is questions about the code...maybe roland can answer.
All this is style, and most refers to code I didnt change.
This is for Roland to decide.
> 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
--
I dont speak for Mellanox
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
