Re: [ofa-general] Re: [PATCH 2.6.30] xprtrdma: The frmr iova_start values are truncated by the nfs rdma client.

[Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>]

 > Trond Myklebust wrote (earlier in this thread):
 > >
 > > All I should need to know is that I can advertise either dma handles or
 > > kernel VAs, and know that I can choose between two functions, say,
 > > ib_send_wr_fastreg_dma_init() and ib_send_wr_fastreg_kva_init() to
 > > initialise the ib_send_wr structure correctly.

I skimmed the earlier thread, and I have to say that I don't quite see
what the problem with assigning things to a u64 directly is.  You can
use any address you want, and I don't quite understand why using the
correct cast to avoid sign extension or truncation problems is such a
big maintenance burden?

The code below really just looks like obfuscation to me -- are we going
to want to add something like

/**
 * ib_init_fast_reg_iova_start_u64 - initializes the iova_start field
 *   based on a 64-bit address supplied by the user.
 * @wr - struct ib_send_wr pointer to be initialized
 * @addr - void * address to be used as the iova_start
 */
static inline void ib_init_fast_reg_iova_start_kva(struct ib_send_wr *wr,
						   u64 addr)
{
	wr->wr.fast_reg.iova_start = addr;
}

next, to make sure we don't get confused about assigning a u64 to a u64?
It all looks a bit overcomplicated to me.

 - R.

 > /**
 > + * ib_init_fast_reg_iova_start_dma - initializes the iova_start field
 > + *   based on a dma address supplied by the user.
 > + * @wr - struct ib_send_wr pointer to be initialized
 > + * @addr - dma_addr_t value to be used as the iova_start
 > + */
 > +static inline void ib_init_fast_reg_iova_start_dma(struct ib_send_wr *wr,
 > +                                                  dma_addr_t addr)
 > +{
 > +       wr->wr.fast_reg.iova_start = addr;
 > +}
 > +
 > +/**
 > + * ib_init_fast_reg_iova_start_kva - initializes the iova_start field
 > + *   based on a kernel virtual address supplied by the user.
 > + * @wr - struct ib_send_wr pointer to be initialized
 > + * @addr - void * address to be used as the iova_start
 > + */
 > +static inline void ib_init_fast_reg_iova_start_kva(struct ib_send_wr *wr,
 > +                                                  void *addr)
 > +{
 > +       wr->wr.fast_reg.iova_start = (unsigned long)addr;
 > +}
 > +
 > +/**
 >  * ib_alloc_mw - Allocates a memory window.
 >  * @pd: The protection domain associated with the memory window.
 >  */