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

[Eli Cohen]

On 1/24/08, Roland Dreier <rdreier at cisco.com> wrote:
> 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,

The following two subsequent ifs gurantees that.

               if (ind < 0) {
                        err = -1;
                        *bad_wr = wr;
                        break;
                }

                wqe       = get_wqe(srq, ind);
                next_ind  = *wqe_to_link(wqe);

                if (next_ind < 0) {
                        err = -1;
                        *bad_wr = wr;
                        break;
                }



> 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.

The two ifs I mentioned above guarantee that the consumer cannot post
to the last entry, and they also enable to assert the requirement that
all posted WQEs have a valid NDA field.

> 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.
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>