[ofa-general] Re: [PATCH] ib/limthca: Remove an always true condition

Roland Dreier rdreier at cisco.com
Thu Jan 24 13:00:37 PST 2008


Thanks for splitting this up... it makes review much easier, and
indeed it seems either there is a bug in the existing code, or this
patch is wrong:

 > -	if (srq->first_free >= 0)
 > -		*wqe_to_link(get_wqe(srq, srq->last_free)) = ind;
 > -	else
 > -		srq->first_free = ind;
 > -
 > +	*wqe_to_link(get_wqe(srq, srq->last_free)) = ind;

why is first_free always >= 0?  I don't see anything that guarantees
that, and in fact mthca_tavor_post_srq_recv and mthca_arbel_post_srq_recv
both have code:

		ind = srq->first_free;

		if (ind < 0) {

so if first_free is always non-negative, we could also delete these
checks from the fast path.  I do see the SRQ create code adds a spare entry:

	srq->max     = align_queue_size(pd->context, attr->attr.max_wr, 1);

but it seems we don't prevent the consumer from using this entry.
However I can't recreate the reasoning why we need the spare entry...

The most straightforward fix is to change the check for the SRQ being
full in the post_srq_recv functions so that it keeps the spare entry,
but I'd like to understand the code again first ;)

 - R.



More information about the general mailing list