Re: [PATCH 8/8] IB/srp: Drain the send queue before destroying a QP

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On Sun, Feb 12, 2017 at 08:11:53PM +0000, Bart Van Assche wrote:
> On Sun, 2017-02-12 at 19:19 +0200, Leon Romanovsky wrote:
> > On Fri, Feb 10, 2017 at 03:56:11PM -0800, Bart Van Assche wrote:
> > > -static void srp_destroy_qp(struct ib_qp *qp)
> > > +static void srp_destroy_qp(struct srp_rdma_ch *ch, struct ib_qp *qp)
> > >  {
> > > -	ib_drain_rq(qp);
> > > +	spin_lock_irq(&ch->lock);
> > > +	ib_process_cq_direct(ch->send_cq, -1);
> >
> > I see that you are already using "-1" in your code, but the comments in the
> > ib_process_cq_direct states that no new code should use "-1".
> >
> >  61  * Note: for compatibility reasons -1 can be passed in %budget for unlimited
> >  62  * polling.  Do not use this feature in new code, it will be removed soon.
> >  63  */
> >  64 int ib_process_cq_direct(struct ib_cq *cq, int budget)
>
> Although it is possible to avoid passing -1 as 'budget' by passing a number
> that is at least as large as the number of expected completions, it would
> make it harder to verify the SRP initiator driver. So I propose to modify
> the comment above ib_process_cq_direct().

I don't know,
It seems like an easiest approach is to change the comment especially while
SRP is the only one user of this call. However ability to properly calculate
number of expected completions and compare it while doing destroy_qp is
a valuable thing for correctness too.

Thanks

>
> Bart.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]