Re: [rdma-core v3 8/9] libbnxt_re: Add support for atomic operations

Bart Van Assche
2017-03-16T16:07:38Z

On Thu, 2017-03-16 at 21:24 +0530, Devesh Sharma wrote:
> On Thu, Mar 16, 2017 at 12:45 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, Mar 15, 2017 at 06:37:32AM -0400, Devesh Sharma wrote:
> > > @@ -1116,6 +1119,44 @@ static int bnxt_re_build_rdma_sqe(struct bnxt_re_qp *qp, void *wqe,
> > > return len;
> > > }
> > >
> > > +static int bnxt_re_build_cns_sqe(struct bnxt_re_qp *qp, void *wqe,
> > > + struct ibv_send_wr *wr)
> > > +{
> > > + struct bnxt_re_bsqe *hdr = wqe;
> > > + struct bnxt_re_atomic *sqe = ((void *)wqe +
> > > + sizeof(struct bnxt_re_bsqe));
> > > + int len;
> > > +
> > > + len = bnxt_re_build_send_sqe(qp, wqe, wr, false);
> > > + hdr->key_immd = wr->wr.atomic.rkey;
> > > + sqe->rva_lo = lower_32_bits(wr->wr.atomic.remote_addr);
> > > + sqe->rva_hi = upper_32_bits(wr->wr.atomic.remote_addr);
> > > + sqe->cmp_dt_lo = lower_32_bits(wr->wr.atomic.compare_add);
> > > + sqe->cmp_dt_hi = upper_32_bits(wr->wr.atomic.compare_add);
> > > + sqe->swp_dt_lo = lower_32_bits(wr->wr.atomic.swap);
> > > + sqe->swp_dt_hi = upper_32_bits(wr->wr.atomic.swap);
> > > +
> >
> > Sorry for my question, but why do you need to separate uint64_t variable
> > to two __u32? Why can't you declare one __u64?
>
> It may help 32 bit CPU during byte order conversion.

Does that mean that you consider optimizing for 32-bit CPUs more important than
optimizing for 64-bit CPUs? Does this also mean that you expect that the macros
you introduced in your driver to be more efficient than the glibc endianness
conversion macros? Sorry but this sounds like a weird explanation to me. I agree
with Leon that it would be a significant improvement if 64-bit data types would
be used instead of using numerous lower_32_bits() / upper_32_bits() calls.

Bart.--