| Message ID | 1447691861-3796-3-git-send-email-sagig@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 11/16/2015 6:37 PM, Sagi Grimberg wrote:
> This is a pre-step before adding remote invalidate support.
Wouldn't that introduce performance regression on ConnectX3 devices?
Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
> + if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) { > + iser_info("FastReg supported, using FastReg for registration\n"); > + device->reg_ops = &fastreg_ops; > + } else > if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr && Should an 'else if'. If the line gets to long it might be time to add a rdma_cap_fmr() helper to ib_verbs.h -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 11/16/2015 6:37 PM, Sagi Grimberg wrote: >> This is a pre-step before adding remote invalidate support. > > Wouldn't that introduce performance regression on ConnectX3 devices? With remote invalidate it shouldn't make much of a difference. Plus, I'd really like us to start phasing out from FMRs... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17/11/2015 10:57, Christoph Hellwig wrote: >> + if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) { >> + iser_info("FastReg supported, using FastReg for registration\n"); >> + device->reg_ops = &fastreg_ops; >> + } else >> if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr && > > Should an 'else if'. If the line gets to long it might be time to add a > rdma_cap_fmr() helper to ib_verbs.h OK, I will. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/17/2015 11:35 AM, Sagi Grimberg wrote: >> On 11/16/2015 6:37 PM, Sagi Grimberg wrote: >>> This is a pre-step before adding remote invalidate support. >> >> Wouldn't that introduce performance regression on ConnectX3 devices? > > With remote invalidate it shouldn't make much of a difference. Why? the invalidate is just one part of the story, we are doing a mapping on IO submission and CX3 has strong ordering on FRWRs, right? > Plus, I'd really like us to start phasing out from FMRs... We should make sure not to introduce performance regression for HW which has such a big existing install base and is well selling in 2015/16 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Why? the invalidate is just one part of the story, we are doing a > mapping on IO submission > and CX3 has strong ordering on FRWRs, right? Yes, this is correct. We'll test on CX3 to see if this introduces a regression. > We should make sure not to introduce performance regression for HW which > has such a big existing install base and is well selling in 2015/16 You are right. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c index ea765fb9664d..1103aa6e8d00 100644 --- a/drivers/infiniband/ulp/iser/iser_memory.c +++ b/drivers/infiniband/ulp/iser/iser_memory.c @@ -71,15 +71,14 @@ int iser_assign_reg_ops(struct iser_device *device) { struct ib_device_attr *dev_attr = &device->dev_attr; - /* Assign function handles - based on FMR support */ + if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) { + iser_info("FastReg supported, using FastReg for registration\n"); + device->reg_ops = &fastreg_ops; + } else if (device->ib_device->alloc_fmr && device->ib_device->dealloc_fmr && device->ib_device->map_phys_fmr && device->ib_device->unmap_fmr) { iser_info("FMR supported, using FMR for registration\n"); device->reg_ops = &fmr_ops; - } else - if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) { - iser_info("FastReg supported, using FastReg for registration\n"); - device->reg_ops = &fastreg_ops; } else { iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n"); return -1;
This is a pre-step before adding remote invalidate support. Also, every ULP should prefer fastreg over fmr. Signed-off-by: Sagi Grimberg <sagig@mellanox.com> --- drivers/infiniband/ulp/iser/iser_memory.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)