RE: [PATCH 1/3] IB: new common API for draining queues

["Steve Wise" <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>]

> > +/*
> > + * Post a WR and block until its completion is reaped for the SQ.
> > + */
> > +static void __ib_drain_sq(struct ib_qp *qp)
> > +{
> > +       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
> > +       struct ib_drain_cqe sdrain;
> > +       struct ib_send_wr swr = {}, *bad_swr;
> > +       int ret;
> > +
> > +       swr.wr_cqe = &sdrain.cqe;
> > +       sdrain.cqe.done = ib_drain_qp_done;
> > +       init_completion(&sdrain.done);
> > +
> > +       ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
> > +       if (ret) {
> > +               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
> > +               return;
> > +       }
> 
> Should it query qp-state and then modify if needed? I am not liking the
> the extra step which is highly unlikely in a normal usage conditions?
> especially for a shared CQ it is really redundant.
>

Hey Devesh, on a previous round of reviews for this series, I thought you agreed that this was fine? 

See: http://www.spinics.net/lists/linux-rdma/msg33202.html

My personal opinion is that calling query_qp() and only modifying if needed is as much work as just calling modify since ERR->ERR is basically a noop.

Steve.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html