[PATCH 1/2] nvme: enable SG gaps support

[PATCH 1/2] nvme: enable SG gaps support

[Max Gurtovoy]

For controllers that can handle arbitrarily sized bios (e.g advanced
RDMA ctrls) we can allow the block layer to pass us gaps by skip
setting the queue virt_boundary.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c |    5 ++++-
 drivers/nvme/host/nvme.h |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9b3b57f..218a3c1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1252,7 +1252,10 @@ static void nvme_set_queue_limits(struct nvme_ctrl *ctrl,
 	}
 	if (ctrl->quirks & NVME_QUIRK_STRIPE_SIZE)
 		blk_queue_chunk_sectors(q, ctrl->max_hw_sectors);
-	blk_queue_virt_boundary(q, ctrl->page_size - 1);
+
+	if (!ctrl->sg_gaps_support)
+		blk_queue_virt_boundary(q, ctrl->page_size - 1);
+
 	if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
 		vwc = true;
 	blk_queue_write_cache(q, vwc, vwc);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 2aa20e3..ccb895a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -162,6 +162,7 @@ struct nvme_ctrl {
 	struct work_struct scan_work;
 	struct work_struct async_event_work;
 	struct delayed_work ka_work;
+	bool sg_gaps_support;
 
 	/* Power saving configuration */
 	u64 ps_max_latency_us;
-- 
1.7.1

[PATCH 2/2] nvme-rdma: add support for arbitrary sg lists mapping

[Max Gurtovoy]

If the device support arbitrary sg list mapping (device
cap IB_DEVICE_SG_GAPS_REG set) we allocate the memory regions with
IB_MR_TYPE_SG_GAPS and allow the block layer to send us gappy bio
vectors. We use this functionality in iSER driver as well.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/rdma.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 47a479f..2c43e27 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -67,6 +67,7 @@ struct nvme_rdma_qe {
 struct nvme_rdma_request {
 	struct nvme_request	req;
 	struct ib_mr		*mr;
+	enum ib_mr_type		mr_type;
 	struct nvme_rdma_qe	sqe;
 	struct ib_sge		sge[1 + NVME_RDMA_MAX_INLINE_SEGMENTS];
 	u32			num_sge;
@@ -294,7 +295,7 @@ static int nvme_rdma_reinit_request(void *data, struct request *rq)
 
 	ib_dereg_mr(req->mr);
 
-	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
+	req->mr = ib_alloc_mr(dev->pd, req->mr_type,
 			ctrl->max_fr_pages);
 	if (IS_ERR(req->mr)) {
 		ret = PTR_ERR(req->mr);
@@ -348,8 +349,12 @@ static int __nvme_rdma_init_request(struct nvme_rdma_ctrl *ctrl,
 	if (ret)
 		return ret;
 
-	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
-			ctrl->max_fr_pages);
+	if (ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
+		req->mr_type = IB_MR_TYPE_SG_GAPS;
+	else
+		req->mr_type = IB_MR_TYPE_MEM_REG;
+
+	req->mr = ib_alloc_mr(dev->pd, req->mr_type, ctrl->max_fr_pages);
 	if (IS_ERR(req->mr)) {
 		ret = PTR_ERR(req->mr);
 		goto out_free_qe;
@@ -1608,6 +1613,9 @@ static int nvme_rdma_configure_admin_queue(struct nvme_rdma_ctrl *ctrl)
 	ctrl->ctrl.sqsize =
 		min_t(int, NVME_CAP_MQES(ctrl->cap) + 1, ctrl->ctrl.sqsize);
 
+	if (ctrl->device->dev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
+		ctrl->ctrl.sg_gaps_support = true;
+
 	error = nvme_enable_ctrl(&ctrl->ctrl, ctrl->cap);
 	if (error)
 		goto out_cleanup_queue;
-- 
1.7.1

[PATCH 1/2] nvme: enable SG gaps support

[Keith Busch]

On Wed, Apr 12, 2017@01:01:48AM +0300, Max Gurtovoy wrote:
> For controllers that can handle arbitrarily sized bios (e.g advanced
> RDMA ctrls) we can allow the block layer to pass us gaps by skip
> setting the queue virt_boundary.
> 
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>

Looks fine.

Reviewed-by: Keith Busch <keith.busch at intel.com>

[PATCH 1/2] nvme: enable SG gaps support

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH 2/2] nvme-rdma: add support for arbitrary sg lists mapping

[Christoph Hellwig]

> -	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
> +	req->mr = ib_alloc_mr(dev->pd, req->mr_type,
>  			ctrl->max_fr_pages);
>  	if (IS_ERR(req->mr)) {
>  		ret = PTR_ERR(req->mr);
> @@ -348,8 +349,12 @@ static int __nvme_rdma_init_request(struct nvme_rdma_ctrl *ctrl,
>  	if (ret)
>  		return ret;
>  
> -	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
> -			ctrl->max_fr_pages);
> +	if (ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
> +		req->mr_type = IB_MR_TYPE_SG_GAPS;
> +	else
> +		req->mr_type = IB_MR_TYPE_MEM_REG;

Why do we need the req->mr_type member?  Can't we just check the device
aps in the reinit path as well?

[PATCH 2/2] nvme-rdma: add support for arbitrary sg lists mapping

[Max Gurtovoy]

On 4/12/2017 8:17 PM, Christoph Hellwig wrote:
>> -	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
>> +	req->mr = ib_alloc_mr(dev->pd, req->mr_type,
>>  			ctrl->max_fr_pages);
>>  	if (IS_ERR(req->mr)) {
>>  		ret = PTR_ERR(req->mr);
>> @@ -348,8 +349,12 @@ static int __nvme_rdma_init_request(struct nvme_rdma_ctrl *ctrl,
>>  	if (ret)
>>  		return ret;
>>
>> -	req->mr = ib_alloc_mr(dev->pd, IB_MR_TYPE_MEM_REG,
>> -			ctrl->max_fr_pages);
>> +	if (ibdev->attrs.device_cap_flags & IB_DEVICE_SG_GAPS_REG)
>> +		req->mr_type = IB_MR_TYPE_SG_GAPS;
>> +	else
>> +		req->mr_type = IB_MR_TYPE_MEM_REG;
>
> Why do we need the req->mr_type member?  Can't we just check the device
> aps in the reinit path as well?
>

I guess we can. Is it better to use an helper func to avoid code 
duplication ? if so, maybe we can do a general fuctions in verbs.h so 
iser can use it too ?

[PATCH 2/2] nvme-rdma: add support for arbitrary sg lists mapping

[Christoph Hellwig]

On Thu, Apr 13, 2017@01:58:22AM +0300, Max Gurtovoy wrote:
> I guess we can. Is it better to use an helper func to avoid code 
> duplication ? if so, maybe we can do a general fuctions in verbs.h so iser 
> can use it too ?

Sure, that sounds useful to me.