[openib-general] [PATCH] (fixed) cqe lookup speedup

[Grant Grundler]

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