RE: [RFC PATCH v5 09/16] RDMA/irdma: Implement device supported verb APIs

["Saleem, Shiraz" <shiraz.saleem@intel.com>]

> Subject: Re: [RFC PATCH v5 09/16] RDMA/irdma: Implement device supported
> verb APIs
> 
> On Fri, Apr 17, 2020 at 10:12:44AM -0700, Jeff Kirsher wrote:
> > From: Mustafa Ismail <mustafa.ismail@intel.com>
> >
> > Implement device supported verb APIs. The supported APIs vary based on
> > the underlying transport the ibdev is registered as (i.e. iWARP or
> > RoCEv2).
> >
> > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > ---
> >  drivers/infiniband/hw/irdma/verbs.c     | 4555 +++++++++++++++++++++++
> >  drivers/infiniband/hw/irdma/verbs.h     |  213 ++
> >  include/uapi/rdma/ib_user_ioctl_verbs.h |    1 +
> >  3 files changed, 4769 insertions(+)
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.c
> >  create mode 100644 drivers/infiniband/hw/irdma/verbs.h
> 
> <...>
> 
> > +static int irdma_destroy_qp(struct ib_qp *ibqp, struct ib_udata
> > +*udata) {
> > +	struct irdma_qp *iwqp = to_iwqp(ibqp);
> > +
> > +	iwqp->destroyed = 1;
> > +	if (iwqp->ibqp_state >= IB_QPS_INIT && iwqp->ibqp_state <
> IB_QPS_RTS)
> > +		irdma_next_iw_state(iwqp, IRDMA_QP_STATE_ERROR, 0, 0, 0);
> > +
> > +	if (!iwqp->user_mode) {
> > +		if (iwqp->iwscq) {
> > +			irdma_clean_cqes(iwqp, iwqp->iwscq);
> > +			if (iwqp->iwrcq != iwqp->iwscq)
> > +				irdma_clean_cqes(iwqp, iwqp->iwrcq);
> > +		}
> > +	}
> > +
> > +	irdma_remove_push_mmap_entries(iwqp);
> > +	irdma_free_lsmm_rsrc(iwqp);
> > +	irdma_rem_ref(&iwqp->ibqp);
> 
> No, please ensure that call to destroy_qp is kfree QP without any need in reference
> counting. We need this to move QP allocation to be IB/core responsibility. I hope
> that all other verbs objects (with MR as
> exception) follow the same pattern: create->kzalloc->destroy>kfree.

Yes. I did see the other verb objects allocation move to IB core
responsibility but not QP. Since we are headed in that direction,
I do think it's a reasonable expectation to make destroy QP
synchronous in providers. We ll look to change it in next rev.

Thank you Leon for taking the time to review and provide
feedback.

Shiraz