Re: Re: Re: Re: Re: [PATCH for-next] RDMA/siw: fix SQ/RQ drain logic to support ib_drain_qp

["Bernard Metzler" <BMT@zurich.ibm.com>]

-----"Krishnamraju Eraparaju" <krishna2@chelsio.com> wrote: -----

>To: "Bernard Metzler" <BMT@zurich.ibm.com>
>From: "Krishnamraju Eraparaju" <krishna2@chelsio.com>
>Date: 10/04/2019 06:57AM
>Cc: "jgg@ziepe.ca" <jgg@ziepe.ca>, "linux-rdma@vger.kernel.org"
><linux-rdma@vger.kernel.org>, "Potnuri Bharat Teja"
><bharat@chelsio.com>, "Nirranjan Kirubaharan" <nirranjan@chelsio.com>
>Subject: [EXTERNAL] Re: Re: Re: Re: [PATCH for-next] RDMA/siw: fix
>SQ/RQ drain logic to support ib_drain_qp
>
>On Thursday, October 10/03/19, 2019 at 14:03:05 +0000, Bernard
>Metzler wrote:
>> There are other reasons why the generic
>> __ib_drain_sq() may fail. A CQ overflow is one
>> such candidate. Failures are not handled by the ULP,
>> since calling a void function.
>The function description of ib_drain_qp() says:
> * The caller must:
> *
> * ensure there is room in the CQ(s), SQ, and RQ for drain work
>requests
> * and completions.
> *
> * allocate the CQs using ib_alloc_cq().
> *
> * ensure that there are no other contexts that are posting WRs
> * concurrently.
> * Otherwise the drain is not guaranteed.
> */
>
Yes, I know. Imho, this guarantee falls into the same category
as assuming a sane ULP which will not try to change the QP state
at the same time while calling for sq_drain. A CQ overflow  would
be a miscalculation of its size by the ULP. A drain_sq in parallel
with a modify_qp call just another misbehaving..? Anyway, I think
you are right, let's handle explicitly all cases we can handle.

>
>So, it looks like ULP has to check for available CQs before calling
>ib_drain_xx(). 
>> 
>> At the other hand, we know that if we have reached
>> ERROR state, the QP will never escape back to become
>> full functional; ERROR is the QP's final state.
>> 
>> So we could do an extra check if we cannot get
>> the state lock - if we are already in ERROR. And
>> if yes, complete immediately there as well.
>> 
>> I can change the patch accordingly. Makes sense?
>Yes, I think addressing this would make the fix complete.
>
sent.

I'll be away whole next week from tonight on.

Thanks
Bernard.

>Thanks,
>Krishna.
>
>