On Mon, Apr 15, 2019 at 08:31:03AM -0400, Dennis Dalessandro wrote:
> On 4/12/2019 11:43 AM, Leon Romanovsky wrote:
> > On Fri, Apr 12, 2019 at 06:41:42AM -0700, Dennis Dalessandro wrote:
> > > From: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> > >
> > > The reference count adjustments on reference count completion
> > > are open coded throughout.
> > >
> > > Add a routine to do all reference count adjustments and use.
> > >
> > > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
> > > Signed-off-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>
> > >
> > > ---
> > > For some reason patch didn't make the list or patchworks. This
> > > is just a re-send. All the rest seem to have made it.
> > > ---
> > > drivers/infiniband/hw/hfi1/rc.c | 4 ++--
> > > drivers/infiniband/hw/qib/qib_rc.c | 4 ++--
> > > drivers/infiniband/sw/rdmavt/qp.c | 9 ++-------
> > > include/rdma/rdmavt_qp.h | 14 ++++++++++++++
> > > 4 files changed, 20 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
> > > index 5ba39a9..a922edc 100644
> > > --- a/drivers/infiniband/hw/hfi1/rc.c
> > > +++ b/drivers/infiniband/hw/hfi1/rc.c
> > > @@ -1834,7 +1834,7 @@ void hfi1_rc_send_complete(struct rvt_qp *qp, struct hfi1_opa_header *opah)
> > > qp->s_last = s_last;
> > > /* see post_send() */
> > > barrier();
> > > - rvt_put_swqe(wqe);
> > > + rvt_put_qp_swqe(qp, wqe);
> > > rvt_qp_swqe_complete(qp,
> > > wqe,
> > > ib_hfi1_wc_opcode[wqe->wr.opcode],
> > > @@ -1882,7 +1882,7 @@ struct rvt_swqe *do_rc_completion(struct rvt_qp *qp,
> > > u32 s_last;
> > >
> > > trdma_clean_swqe(qp, wqe);
> > > - rvt_put_swqe(wqe);
> > > + rvt_put_qp_swqe(qp, wqe);
> > > rvt_qp_wqe_unreserve(qp, wqe);
> > > s_last = qp->s_last;
> > > trace_hfi1_qp_send_completion(qp, wqe, s_last);
> > > diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
> > > index 50dd981..2ac4c67 100644
> > > --- a/drivers/infiniband/hw/qib/qib_rc.c
> > > +++ b/drivers/infiniband/hw/qib/qib_rc.c
> > > @@ -933,7 +933,7 @@ void qib_rc_send_complete(struct rvt_qp *qp, struct ib_header *hdr)
> > > qp->s_last = s_last;
> > > /* see post_send() */
> > > barrier();
> > > - rvt_put_swqe(wqe);
> > > + rvt_put_qp_swqe(qp, wqe);
> > > rvt_qp_swqe_complete(qp,
> > > wqe,
> > > ib_qib_wc_opcode[wqe->wr.opcode],
> > > @@ -975,7 +975,7 @@ static inline void update_last_psn(struct rvt_qp *qp, u32 psn)
> > > qib_cmp24(qp->s_sending_psn, qp->s_sending_hpsn) > 0) {
> > > u32 s_last;
> > >
> > > - rvt_put_swqe(wqe);
> > > + rvt_put_qp_swqe(qp, wqe);
> > > s_last = qp->s_last;
> > > if (++s_last >= qp->s_size)
> > > s_last = 0;
> > > diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> > > index 2460303..31a2e65 100644
> > > --- a/drivers/infiniband/sw/rdmavt/qp.c
> > > +++ b/drivers/infiniband/sw/rdmavt/qp.c
> > > @@ -623,10 +623,7 @@ static void rvt_clear_mr_refs(struct rvt_qp *qp, int clr_sends)
> > > while (qp->s_last != qp->s_head) {
> > > struct rvt_swqe *wqe = rvt_get_swqe_ptr(qp, qp->s_last);
> > >
> > > - rvt_put_swqe(wqe);
> > > - if (qp->allowed_ops == IB_OPCODE_UD)
> > > - atomic_dec(&ibah_to_rvtah(
> > > - wqe->ud_wr.ah)->refcount);
> > > + rvt_put_qp_swqe(qp, wqe);
> > > if (++qp->s_last >= qp->s_size)
> > > qp->s_last = 0;
> > > smp_wmb(); /* see qp_set_savail */
> > > @@ -2683,9 +2680,7 @@ void rvt_send_complete(struct rvt_qp *qp, struct rvt_swqe *wqe,
> > > qp->s_last = last;
> > > /* See post_send() */
> > > barrier();
> > > - rvt_put_swqe(wqe);
> > > - if (qp->allowed_ops == IB_OPCODE_UD)
> > > - atomic_dec(&ibah_to_rvtah(wqe->ud_wr.ah)->refcount);
> > > + rvt_put_qp_swqe(qp, wqe);
> > >
> > > rvt_qp_swqe_complete(qp,
> > > wqe,
> > > diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h
> > > index a00c46a..68e38c2 100644
> > > --- a/include/rdma/rdmavt_qp.h
> > > +++ b/include/rdma/rdmavt_qp.h
> > > @@ -723,6 +723,20 @@ static inline void rvt_mod_retry_timer(struct rvt_qp *qp)
> > > return rvt_mod_retry_timer_ext(qp, 0);
> > > }
> > >
> > > +/**
> > > + * rvt_put_qp_swqe - drop refs held by swqe
> > > + * @qp: the send qp
> > > + * @wqe: the send wqe
> > > + *
> > > + * This drops any references held by the swqe
> > > + */
> > > +static inline void rvt_put_qp_swqe(struct rvt_qp *qp, struct rvt_swqe *wqe)
> > > +{
> > > + rvt_put_swqe(wqe);
> > > + if (qp->allowed_ops == IB_OPCODE_UD)
> > > + atomic_dec(&ibah_to_rvtah(wqe->ud_wr.ah)->refcount);
> > > +}
> >
> > In the context of my allocation change patches, I wanted to ask on which
> > objects should driver perform refcounting?
>
> Are you asking if we really need to do our own refcounting in the driver
> since your patch series went in to for-next?
For this AH code - yes, I don't think that refcounting is needed.
However my question is more general - I need to know in which objects
we can remove refcounting without any worries that I'll break some driver.
Thanks
>
> -Denny
>
>
>
Attachment:
signature.asc
Description: PGP signature
