> -----Original Message----- > From: Jason Gunthorpe [mailto:jgunthorpe@xxxxxxxxxxxxxxxxxxxx] > Sent: Friday, July 24, 2015 11:57 AM > To: Steve Wise > Cc: dledford@xxxxxxxxxx; infinipath@xxxxxxxxx; sagig@xxxxxxxxxxxx; ogerlitz@xxxxxxxxxxxx; roid@xxxxxxxxxxxx; linux- > rdma@xxxxxxxxxxxxxxx; eli@xxxxxxxxxxxx; target-devel@xxxxxxxxxxxxxxx; linux-nfs@xxxxxxxxxxxxxxx; bfields@xxxxxxxxxxxx > Subject: Re: [PATCH V6 9/9] isert: Support iWARP transports using FRMRs > > On Fri, Jul 24, 2015 at 11:19:05AM -0500, Steve Wise wrote: > > Add new register and unregister functions to be used with devices that > > support FRMRs, provide a local dma lkey, yet do not support DIF/PI. > > > > isert_reg_frmr() only needs to use FRMRs for RDMA READ since RDMA WRITE > > can be handled entirely with the local dma lkey. So for RDMA READ, > > it calls isert_reg_read_frmr(). Otherwise is uses the lkey map service > > isert_map_lkey() for RDMA WRITEs. > > > > isert_reg_read_frmr() will create a linked list of WR triplets of the > > form: INV->FRWR->READ. The number of these triplets is dependent on > > the devices fast reg page list length limit. > > That ordering seems strange, surely it should be > > FRWR->READ->INV > > And use IB_WR_RDMA_READ_WITH_INV if possible? > > ACCESS_REMOTE rkey's should not be left open across the FROM_DEVICE > DMA flush. > You're correct. I was thinking to simplify the IO by always invalidating before re-registering. But it does leave the FRMR registered and exposes a security hole. I'll have to rework this. > > /* assign function handlers */ > > - if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > - dev_attr->device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY && > > - dev_attr->device_cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > - device->use_fastreg = 1; > > - device->reg_rdma_mem = isert_reg_frmr_pi; > > - device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + cap_flags = dev_attr->device_cap_flags; > > + if (cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && > > + cap_flags & IB_DEVICE_LOCAL_DMA_LKEY) { > > + if (cap_flags & IB_DEVICE_SIGNATURE_HANDOVER) { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr_pi; > > + device->unreg_rdma_mem = isert_unreg_frmr_pi; > > + } else { > > + device->use_fastreg = 1; > > + device->reg_rdma_mem = isert_reg_frmr; > > + device->unreg_rdma_mem = isert_unreg_frmr; > > + } > > The use of FRWR for RDMA READ should be iWarp specific, IB shouldn't > pay that overhead. I am expecting to see a cap_rdma_read_rkey or > something in here ? > Ok. But cap_rdma_read_rkey() doesn't really describe the requirement. The requirement is rkey + REMOTE_WRITE. So it is more like rdma_cap_read_requires_remote_write() which is ugly and too long (but descriptive)... -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html