* [PATCH 01/10] IB/core: Guarantee that a local_dma_lkey is available
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-23 10:47 ` Sagi Grimberg
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
` (8 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
Every single ULP requires a local_dma_lkey to do anything with
a QP, so let us ensure one exists for every PD created.
If the driver can supply a global local_dma_lkey then use that, otherwise
ask the driver to create a local use all physical memory MR associated
with the new PD.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
Acked-by: Christoph Hellwig <hch@infradead.org>
---
drivers/infiniband/core/verbs.c | 40 +++++++++++++++++++++++++++++++++++-----
include/rdma/ib_verbs.h | 2 ++
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index bac3fb406a74..1ddf06314f36 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -213,24 +213,54 @@ EXPORT_SYMBOL(rdma_port_get_link_layer);
/* Protection domains */
+/* Return a pd for in-kernel use that has a local_dma_lkey which provides
+ local access to all physical memory. */
struct ib_pd *ib_alloc_pd(struct ib_device *device)
{
struct ib_pd *pd;
+ struct ib_device_attr devattr;
+ int rc;
+
+ rc = ib_query_device(device, &devattr);
+ if (rc)
+ return ERR_PTR(rc);
pd = device->alloc_pd(device, NULL, NULL);
+ if (IS_ERR(pd))
+ return pd;
+
+ pd->device = device;
+ pd->uobject = NULL;
+ pd->local_mr = NULL;
+ atomic_set(&pd->usecnt, 0);
+
+ if (devattr.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
+ pd->local_dma_lkey = device->local_dma_lkey;
+ else {
+ struct ib_mr *mr;
+
+ mr = ib_get_dma_mr(pd, IB_ACCESS_LOCAL_WRITE);
+ if (IS_ERR(mr)) {
+ ib_dealloc_pd(pd);
+ return (struct ib_pd *)mr;
+ }
- if (!IS_ERR(pd)) {
- pd->device = device;
- pd->uobject = NULL;
- atomic_set(&pd->usecnt, 0);
+ pd->local_mr = mr;
+ pd->local_dma_lkey = pd->local_mr->lkey;
}
-
return pd;
}
+
EXPORT_SYMBOL(ib_alloc_pd);
int ib_dealloc_pd(struct ib_pd *pd)
{
+ if (pd->local_mr) {
+ if (ib_dereg_mr(pd->local_mr))
+ return -EBUSY;
+ pd->local_mr = NULL;
+ }
+
if (atomic_read(&pd->usecnt))
return -EBUSY;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 986fddb08579..cfda95d7b067 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1255,6 +1255,8 @@ struct ib_pd {
struct ib_device *device;
struct ib_uobject *uobject;
atomic_t usecnt; /* count all resources */
+ struct ib_mr *local_mr;
+ u32 local_dma_lkey;
};
struct ib_xrcd {
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 01/10] IB/core: Guarantee that a local_dma_lkey is available
2015-07-22 23:34 ` [PATCH 01/10] IB/core: Guarantee that a local_dma_lkey is available Jason Gunthorpe
@ 2015-07-23 10:47 ` Sagi Grimberg
2015-07-23 18:36 ` Jason Gunthorpe
0 siblings, 1 reply; 31+ messages in thread
From: Sagi Grimberg @ 2015-07-23 10:47 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
On 7/23/2015 2:34 AM, Jason Gunthorpe wrote:
> Every single ULP requires a local_dma_lkey to do anything with
> a QP, so let us ensure one exists for every PD created.
>
> If the driver can supply a global local_dma_lkey then use that, otherwise
> ask the driver to create a local use all physical memory MR associated
> with the new PD.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Reviewed-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
> Acked-by: Christoph Hellwig <hch@infradead.org>
> ---
> drivers/infiniband/core/verbs.c | 40 +++++++++++++++++++++++++++++++++++-----
> include/rdma/ib_verbs.h | 2 ++
> 2 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index bac3fb406a74..1ddf06314f36 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -213,24 +213,54 @@ EXPORT_SYMBOL(rdma_port_get_link_layer);
>
> /* Protection domains */
>
> +/* Return a pd for in-kernel use that has a local_dma_lkey which provides
> + local access to all physical memory. */
Why not kdoc style? we need to move the ib_verbs.h kdocs here anyway.
Might be a good chance to do that for ib_alloc_pd().
> struct ib_pd *ib_alloc_pd(struct ib_device *device)
> {
> struct ib_pd *pd;
> + struct ib_device_attr devattr;
> + int rc;
> +
> + rc = ib_query_device(device, &devattr);
> + if (rc)
> + return ERR_PTR(rc);
>
> pd = device->alloc_pd(device, NULL, NULL);
> + if (IS_ERR(pd))
> + return pd;
> +
> + pd->device = device;
> + pd->uobject = NULL;
> + pd->local_mr = NULL;
> + atomic_set(&pd->usecnt, 0);
> +
> + if (devattr.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
> + pd->local_dma_lkey = device->local_dma_lkey;
> + else {
> + struct ib_mr *mr;
> +
> + mr = ib_get_dma_mr(pd, IB_ACCESS_LOCAL_WRITE);
> + if (IS_ERR(mr)) {
> + ib_dealloc_pd(pd);
> + return (struct ib_pd *)mr;
> + }
>
> - if (!IS_ERR(pd)) {
> - pd->device = device;
> - pd->uobject = NULL;
> - atomic_set(&pd->usecnt, 0);
> + pd->local_mr = mr;
> + pd->local_dma_lkey = pd->local_mr->lkey;
> }
> -
> return pd;
> }
> +
> EXPORT_SYMBOL(ib_alloc_pd);
You have an extra newline here.
>
> int ib_dealloc_pd(struct ib_pd *pd)
> {
> + if (pd->local_mr) {
> + if (ib_dereg_mr(pd->local_mr))
> + return -EBUSY;
> + pd->local_mr = NULL;
> + }
> +
> if (atomic_read(&pd->usecnt))
> return -EBUSY;
>
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 986fddb08579..cfda95d7b067 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -1255,6 +1255,8 @@ struct ib_pd {
> struct ib_device *device;
> struct ib_uobject *uobject;
> atomic_t usecnt; /* count all resources */
> + struct ib_mr *local_mr;
> + u32 local_dma_lkey;
Maybe its better to place the local_dma_lkey in the first cacheline as
it is normally accessed in the hot path?
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 01/10] IB/core: Guarantee that a local_dma_lkey is available
2015-07-23 10:47 ` Sagi Grimberg
@ 2015-07-23 18:36 ` Jason Gunthorpe
0 siblings, 0 replies; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-23 18:36 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Doug Ledford, linux-rdma, Amir Vadai, Andy Grover,
Bart Van Assche, Chien Yen, Christoph Hellwig, Dominique Martinet,
Eli Cohen, Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov,
Or Gerlitz, Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr,
Tom Tucker, Zach Brown, rds-devel, target-devel, v9fs-developer
On Thu, Jul 23, 2015 at 01:47:02PM +0300, Sagi Grimberg wrote:
> >+/* Return a pd for in-kernel use that has a local_dma_lkey which provides
> >+ local access to all physical memory. */
>
> Why not kdoc style? we need to move the ib_verbs.h kdocs here anyway.
> Might be a good chance to do that for ib_alloc_pd().
Right, took care of these.
> >diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> >index 986fddb08579..cfda95d7b067 100644
> >+++ b/include/rdma/ib_verbs.h
> >@@ -1255,6 +1255,8 @@ struct ib_pd {
> > struct ib_device *device;
> > struct ib_uobject *uobject;
> > atomic_t usecnt; /* count all resources */
> >+ struct ib_mr *local_mr;
> >+ u32 local_dma_lkey;
>
> Maybe its better to place the local_dma_lkey in the first cacheline as
> it is normally accessed in the hot path?
Sure, but it doesn't matter too much as it is probably the only hot
item in the pd... At the very least is avoids computing an EA...
I've posted an updated series on my github, none of this is a
functional change, so I'm going to continue to wait for
testing/reviewing of the various pieces before reposting such a big
series.
https://github.com/jgunthorpe/linux/commits/remove-ib_get_dma_mr
Thanks,
Jason
^ permalink raw reply [flat|nested] 31+ messages in thread
[parent not found: <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* [PATCH 02/10] IB/mad: Remove ib_get_dma_mr calls
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 03/10] IB/ipoib: " Jason Gunthorpe
` (2 subsequent siblings)
3 siblings, 0 replies; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
target-devel-u79uwXL29TY76Z2rM5mHXA,
v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
drivers/infiniband/core/mad.c | 26 +++-----------------------
drivers/infiniband/core/mad_priv.h | 1 -
include/rdma/ib_mad.h | 1 -
3 files changed, 3 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index a4b1466c1bf6..3c5fa8dc5ba7 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -338,13 +338,6 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
goto error1;
}
- mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp->pd,
- IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(mad_agent_priv->agent.mr)) {
- ret = ERR_PTR(-ENOMEM);
- goto error2;
- }
-
if (mad_reg_req) {
reg_req = kmemdup(mad_reg_req, sizeof *reg_req, GFP_KERNEL);
if (!reg_req) {
@@ -429,8 +422,6 @@ error4:
spin_unlock_irqrestore(&port_priv->reg_lock, flags);
kfree(reg_req);
error3:
- ib_dereg_mr(mad_agent_priv->agent.mr);
-error2:
kfree(mad_agent_priv);
error1:
return ret;
@@ -590,7 +581,6 @@ static void unregister_mad_agent(struct ib_mad_agent_private *mad_agent_priv)
wait_for_completion(&mad_agent_priv->comp);
kfree(mad_agent_priv->reg_req);
- ib_dereg_mr(mad_agent_priv->agent.mr);
kfree(mad_agent_priv);
}
@@ -1037,7 +1027,7 @@ struct ib_mad_send_buf * ib_create_send_mad(struct ib_mad_agent *mad_agent,
mad_send_wr->mad_agent_priv = mad_agent_priv;
mad_send_wr->sg_list[0].length = hdr_len;
- mad_send_wr->sg_list[0].lkey = mad_agent->mr->lkey;
+ mad_send_wr->sg_list[0].lkey = mad_agent->qp->pd->local_dma_lkey;
/* OPA MADs don't have to be the full 2048 bytes */
if (opa && base_version == OPA_MGMT_BASE_VERSION &&
@@ -1046,7 +1036,7 @@ struct ib_mad_send_buf * ib_create_send_mad(struct ib_mad_agent *mad_agent,
else
mad_send_wr->sg_list[1].length = mad_size - hdr_len;
- mad_send_wr->sg_list[1].lkey = mad_agent->mr->lkey;
+ mad_send_wr->sg_list[1].lkey = mad_agent->qp->pd->local_dma_lkey;
mad_send_wr->send_wr.wr_id = (unsigned long) mad_send_wr;
mad_send_wr->send_wr.sg_list = mad_send_wr->sg_list;
@@ -2884,7 +2874,7 @@ static int ib_mad_post_receive_mads(struct ib_mad_qp_info *qp_info,
struct ib_mad_queue *recv_queue = &qp_info->recv_queue;
/* Initialize common scatter list fields */
- sg_list.lkey = (*qp_info->port_priv->mr).lkey;
+ sg_list.lkey = qp_info->port_priv->pd->local_dma_lkey;
/* Initialize common receive WR fields */
recv_wr.next = NULL;
@@ -3200,13 +3190,6 @@ static int ib_mad_port_open(struct ib_device *device,
goto error4;
}
- port_priv->mr = ib_get_dma_mr(port_priv->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(port_priv->mr)) {
- dev_err(&device->dev, "Couldn't get ib_mad DMA MR\n");
- ret = PTR_ERR(port_priv->mr);
- goto error5;
- }
-
if (has_smi) {
ret = create_mad_qp(&port_priv->qp_info[0], IB_QPT_SMI);
if (ret)
@@ -3247,8 +3230,6 @@ error8:
error7:
destroy_mad_qp(&port_priv->qp_info[0]);
error6:
- ib_dereg_mr(port_priv->mr);
-error5:
ib_dealloc_pd(port_priv->pd);
error4:
ib_destroy_cq(port_priv->cq);
@@ -3283,7 +3264,6 @@ static int ib_mad_port_close(struct ib_device *device, int port_num)
destroy_workqueue(port_priv->wq);
destroy_mad_qp(&port_priv->qp_info[1]);
destroy_mad_qp(&port_priv->qp_info[0]);
- ib_dereg_mr(port_priv->mr);
ib_dealloc_pd(port_priv->pd);
ib_destroy_cq(port_priv->cq);
cleanup_recv_queue(&port_priv->qp_info[1]);
diff --git a/drivers/infiniband/core/mad_priv.h b/drivers/infiniband/core/mad_priv.h
index 5be89f98928f..4a4f7aad0978 100644
--- a/drivers/infiniband/core/mad_priv.h
+++ b/drivers/infiniband/core/mad_priv.h
@@ -199,7 +199,6 @@ struct ib_mad_port_private {
int port_num;
struct ib_cq *cq;
struct ib_pd *pd;
- struct ib_mr *mr;
spinlock_t reg_lock;
struct ib_mad_mgmt_version_table version[MAX_MGMT_VERSION];
diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
index c8422d5a5a91..1f27023c919a 100644
--- a/include/rdma/ib_mad.h
+++ b/include/rdma/ib_mad.h
@@ -388,7 +388,6 @@ enum {
struct ib_mad_agent {
struct ib_device *device;
struct ib_qp *qp;
- struct ib_mr *mr;
ib_mad_recv_handler recv_handler;
ib_mad_send_handler send_handler;
ib_mad_snoop_handler snoop_handler;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/10] IB/ipoib: Remove ib_get_dma_mr calls
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-07-22 23:34 ` [PATCH 02/10] IB/mad: Remove ib_get_dma_mr calls Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 07/10] iser-target: " Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 08/10] IB/srp: Use pd->local_dma_lkey Jason Gunthorpe
3 siblings, 0 replies; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
target-devel-u79uwXL29TY76Z2rM5mHXA,
v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 1 -
drivers/infiniband/ulp/ipoib/ipoib_cm.c | 2 +-
drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 18 +++---------------
3 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index bd94b0a6e9e5..731e540a37df 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -342,7 +342,6 @@ struct ipoib_dev_priv {
u16 pkey;
u16 pkey_index;
struct ib_pd *pd;
- struct ib_mr *mr;
struct ib_cq *recv_cq;
struct ib_cq *send_cq;
struct ib_qp *qp;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index cf32a778e7d0..d1c217403c6d 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -332,7 +332,7 @@ static void ipoib_cm_init_rx_wr(struct net_device *dev,
int i;
for (i = 0; i < priv->cm.num_frags; ++i)
- sge[i].lkey = priv->mr->lkey;
+ sge[i].lkey = priv->pd->local_dma_lkey;
sge[0].length = IPOIB_CM_HEAD_SIZE;
for (i = 1; i < priv->cm.num_frags; ++i)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
index 851c8219d501..8c451983d8a5 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
@@ -152,12 +152,6 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca)
return -ENODEV;
}
- priv->mr = ib_get_dma_mr(priv->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(priv->mr)) {
- printk(KERN_WARNING "%s: ib_get_dma_mr failed\n", ca->name);
- goto out_free_pd;
- }
-
/*
* the various IPoIB tasks assume they will never race against
* themselves, so always use a single thread workqueue
@@ -165,7 +159,7 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca)
priv->wq = create_singlethread_workqueue("ipoib_wq");
if (!priv->wq) {
printk(KERN_WARNING "ipoib: failed to allocate device WQ\n");
- goto out_free_mr;
+ goto out_free_pd;
}
size = ipoib_recvq_size + 1;
@@ -225,13 +219,13 @@ int ipoib_transport_dev_init(struct net_device *dev, struct ib_device *ca)
priv->dev->dev_addr[3] = (priv->qp->qp_num ) & 0xff;
for (i = 0; i < MAX_SKB_FRAGS + 1; ++i)
- priv->tx_sge[i].lkey = priv->mr->lkey;
+ priv->tx_sge[i].lkey = priv->pd->local_dma_lkey;
priv->tx_wr.opcode = IB_WR_SEND;
priv->tx_wr.sg_list = priv->tx_sge;
priv->tx_wr.send_flags = IB_SEND_SIGNALED;
- priv->rx_sge[0].lkey = priv->mr->lkey;
+ priv->rx_sge[0].lkey = priv->pd->local_dma_lkey;
priv->rx_sge[0].length = IPOIB_UD_BUF_SIZE(priv->max_ib_mtu);
priv->rx_wr.num_sge = 1;
@@ -254,9 +248,6 @@ out_free_wq:
destroy_workqueue(priv->wq);
priv->wq = NULL;
-out_free_mr:
- ib_dereg_mr(priv->mr);
-
out_free_pd:
ib_dealloc_pd(priv->pd);
@@ -289,9 +280,6 @@ void ipoib_transport_dev_cleanup(struct net_device *dev)
priv->wq = NULL;
}
- if (ib_dereg_mr(priv->mr))
- ipoib_warn(priv, "ib_dereg_mr failed\n");
-
if (ib_dealloc_pd(priv->pd))
ipoib_warn(priv, "ib_dealloc_pd failed\n");
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/10] iser-target: Remove ib_get_dma_mr calls
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-07-22 23:34 ` [PATCH 02/10] IB/mad: Remove ib_get_dma_mr calls Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 03/10] IB/ipoib: " Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-23 10:49 ` Sagi Grimberg
2015-07-22 23:34 ` [PATCH 08/10] IB/srp: Use pd->local_dma_lkey Jason Gunthorpe
3 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
target-devel-u79uwXL29TY76Z2rM5mHXA,
v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
drivers/infiniband/ulp/isert/ib_isert.c | 33 +++++++++++----------------------
drivers/infiniband/ulp/isert/ib_isert.h | 1 -
2 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 771700963127..00d75d977131 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -235,7 +235,7 @@ isert_alloc_rx_descriptors(struct isert_conn *isert_conn)
rx_sg = &rx_desc->rx_sg;
rx_sg->addr = rx_desc->dma_addr;
rx_sg->length = ISER_RX_PAYLOAD_SIZE;
- rx_sg->lkey = device->mr->lkey;
+ rx_sg->lkey = device->pd->local_dma_lkey;
}
isert_conn->rx_desc_head = 0;
@@ -385,22 +385,12 @@ isert_create_device_ib_res(struct isert_device *device)
goto out_cq;
}
- device->mr = ib_get_dma_mr(device->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(device->mr)) {
- ret = PTR_ERR(device->mr);
- isert_err("failed to create dma mr, device %p, ret=%d\n",
- device, ret);
- goto out_mr;
- }
-
/* Check signature cap */
device->pi_capable = dev_attr->device_cap_flags &
IB_DEVICE_SIGNATURE_HANDOVER ? true : false;
return 0;
-out_mr:
- ib_dealloc_pd(device->pd);
out_cq:
isert_free_comps(device);
return ret;
@@ -411,7 +401,6 @@ isert_free_device_ib_res(struct isert_device *device)
{
isert_info("device %p\n", device);
- ib_dereg_mr(device->mr);
ib_dealloc_pd(device->pd);
isert_free_comps(device);
}
@@ -1086,8 +1075,8 @@ isert_create_send_desc(struct isert_conn *isert_conn,
tx_desc->num_sge = 1;
tx_desc->isert_cmd = isert_cmd;
- if (tx_desc->tx_sg[0].lkey != device->mr->lkey) {
- tx_desc->tx_sg[0].lkey = device->mr->lkey;
+ if (tx_desc->tx_sg[0].lkey != device->pd->local_dma_lkey) {
+ tx_desc->tx_sg[0].lkey = device->pd->local_dma_lkey;
isert_dbg("tx_desc %p lkey mismatch, fixing\n", tx_desc);
}
}
@@ -1110,7 +1099,7 @@ isert_init_tx_hdrs(struct isert_conn *isert_conn,
tx_desc->dma_addr = dma_addr;
tx_desc->tx_sg[0].addr = tx_desc->dma_addr;
tx_desc->tx_sg[0].length = ISER_HEADERS_LEN;
- tx_desc->tx_sg[0].lkey = device->mr->lkey;
+ tx_desc->tx_sg[0].lkey = device->pd->local_dma_lkey;
isert_dbg("Setup tx_sg[0].addr: 0x%llx length: %u lkey: 0x%x\n",
tx_desc->tx_sg[0].addr, tx_desc->tx_sg[0].length,
@@ -1143,7 +1132,7 @@ isert_rdma_post_recvl(struct isert_conn *isert_conn)
memset(&sge, 0, sizeof(struct ib_sge));
sge.addr = isert_conn->login_req_dma;
sge.length = ISER_RX_LOGIN_SIZE;
- sge.lkey = isert_conn->device->mr->lkey;
+ sge.lkey = isert_conn->device->pd->local_dma_lkey;
isert_dbg("Setup sge: addr: %llx length: %d 0x%08x\n",
sge.addr, sge.length, sge.lkey);
@@ -1193,7 +1182,7 @@ isert_put_login_tx(struct iscsi_conn *conn, struct iscsi_login *login,
tx_dsg->addr = isert_conn->login_rsp_dma;
tx_dsg->length = length;
- tx_dsg->lkey = isert_conn->device->mr->lkey;
+ tx_dsg->lkey = isert_conn->device->pd->local_dma_lkey;
tx_desc->num_sge = 2;
}
if (!login->login_failed) {
@@ -2210,7 +2199,7 @@ isert_put_response(struct iscsi_conn *conn, struct iscsi_cmd *cmd)
isert_cmd->pdu_buf_len = pdu_len;
tx_dsg->addr = isert_cmd->pdu_buf_dma;
tx_dsg->length = pdu_len;
- tx_dsg->lkey = device->mr->lkey;
+ tx_dsg->lkey = device->pd->local_dma_lkey;
isert_cmd->tx_desc.num_sge = 2;
}
@@ -2338,7 +2327,7 @@ isert_put_reject(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
isert_cmd->pdu_buf_len = ISCSI_HDR_LEN;
tx_dsg->addr = isert_cmd->pdu_buf_dma;
tx_dsg->length = ISCSI_HDR_LEN;
- tx_dsg->lkey = device->mr->lkey;
+ tx_dsg->lkey = device->pd->local_dma_lkey;
isert_cmd->tx_desc.num_sge = 2;
isert_init_send_wr(isert_conn, isert_cmd, send_wr);
@@ -2379,7 +2368,7 @@ isert_put_text_rsp(struct iscsi_cmd *cmd, struct iscsi_conn *conn)
isert_cmd->pdu_buf_len = txt_rsp_len;
tx_dsg->addr = isert_cmd->pdu_buf_dma;
tx_dsg->length = txt_rsp_len;
- tx_dsg->lkey = device->mr->lkey;
+ tx_dsg->lkey = device->pd->local_dma_lkey;
isert_cmd->tx_desc.num_sge = 2;
}
isert_init_send_wr(isert_conn, isert_cmd, send_wr);
@@ -2420,7 +2409,7 @@ isert_build_rdma_wr(struct isert_conn *isert_conn, struct isert_cmd *isert_cmd,
ib_sge->addr = ib_sg_dma_address(ib_dev, tmp_sg) + page_off;
ib_sge->length = min_t(u32, data_left,
ib_sg_dma_len(ib_dev, tmp_sg) - page_off);
- ib_sge->lkey = device->mr->lkey;
+ ib_sge->lkey = device->pd->local_dma_lkey;
isert_dbg("RDMA ib_sge: addr: 0x%llx length: %u lkey: %x\n",
ib_sge->addr, ib_sge->length, ib_sge->lkey);
@@ -2594,7 +2583,7 @@ isert_fast_reg_mr(struct isert_conn *isert_conn,
u32 page_off;
if (mem->dma_nents == 1) {
- sge->lkey = device->mr->lkey;
+ sge->lkey = device->pd->local_dma_lkey;
sge->addr = ib_sg_dma_address(ib_dev, &mem->sg[0]);
sge->length = ib_sg_dma_len(ib_dev, &mem->sg[0]);
isert_dbg("sge: addr: 0x%llx length: %u lkey: %x\n",
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index 9ec23a786c02..6a04ba3c0f72 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -209,7 +209,6 @@ struct isert_device {
int refcount;
struct ib_device *ib_device;
struct ib_pd *pd;
- struct ib_mr *mr;
struct isert_comp *comps;
int comps_used;
struct list_head dev_node;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 07/10] iser-target: Remove ib_get_dma_mr calls
2015-07-22 23:34 ` [PATCH 07/10] iser-target: " Jason Gunthorpe
@ 2015-07-23 10:49 ` Sagi Grimberg
0 siblings, 0 replies; 31+ messages in thread
From: Sagi Grimberg @ 2015-07-23 10:49 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
On 7/23/2015 2:34 AM, Jason Gunthorpe wrote:
> The pd now has a local_dma_lkey member which completely replaces
> ib_get_dma_mr, use it instead.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Looks good.
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/10] IB/srp: Use pd->local_dma_lkey
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
` (2 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 07/10] iser-target: " Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
[not found] ` <1437608083-22898-9-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
3 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
target-devel-u79uwXL29TY76Z2rM5mHXA,
v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
Replace all leys with pd->local_dma_lkey. This driver does not support
iWarp, so this is safe.
The insecure use of ib_get_dma_mr is thus isolated to an rkey, and will
have to be fixed separately.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 267dc4f75502..fb9fed0fac28 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3149,7 +3149,7 @@ static ssize_t srp_create_target(struct device *dev,
target->io_class = SRP_REV16A_IB_IO_CLASS;
target->scsi_host = target_host;
target->srp_host = host;
- target->lkey = host->srp_dev->mr->lkey;
+ target->lkey = host->srp_dev->pd->local_dma_lkey;
target->rkey = host->srp_dev->mr->rkey;
target->cmd_sg_cnt = cmd_sg_entries;
target->sg_tablesize = indirect_sg_entries ? : cmd_sg_entries;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 04/10] IB/mlx4: Remove ib_get_dma_mr calls
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 01/10] IB/core: Guarantee that a local_dma_lkey is available Jason Gunthorpe
[not found] ` <1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 05/10] IB/mlx5: " Jason Gunthorpe
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/infiniband/hw/mlx4/mad.c | 23 ++++-------------------
drivers/infiniband/hw/mlx4/mlx4_ib.h | 1 -
2 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 85a50df2f203..3a897ed3db30 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -580,7 +580,7 @@ int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
list.addr = tun_qp->tx_ring[tun_tx_ix].buf.map;
list.length = sizeof (struct mlx4_rcv_tunnel_mad);
- list.lkey = tun_ctx->mr->lkey;
+ list.lkey = tun_ctx->pd->local_dma_lkey;
wr.wr.ud.ah = ah;
wr.wr.ud.port_num = port;
@@ -1123,7 +1123,7 @@ static int mlx4_ib_post_pv_qp_buf(struct mlx4_ib_demux_pv_ctx *ctx,
sg_list.addr = tun_qp->ring[index].map;
sg_list.length = size;
- sg_list.lkey = ctx->mr->lkey;
+ sg_list.lkey = ctx->pd->local_dma_lkey;
recv_wr.next = NULL;
recv_wr.sg_list = &sg_list;
@@ -1234,7 +1234,7 @@ int mlx4_ib_send_to_wire(struct mlx4_ib_dev *dev, int slave, u8 port,
list.addr = sqp->tx_ring[wire_tx_ix].buf.map;
list.length = sizeof (struct mlx4_mad_snd_buf);
- list.lkey = sqp_ctx->mr->lkey;
+ list.lkey = sqp_ctx->pd->local_dma_lkey;
wr.wr.ud.ah = ah;
wr.wr.ud.port_num = port;
@@ -1817,19 +1817,12 @@ static int create_pv_resources(struct ib_device *ibdev, int slave, int port,
goto err_cq;
}
- ctx->mr = ib_get_dma_mr(ctx->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(ctx->mr)) {
- ret = PTR_ERR(ctx->mr);
- pr_err("Couldn't get tunnel DMA MR (%d)\n", ret);
- goto err_pd;
- }
-
if (ctx->has_smi) {
ret = create_pv_sqp(ctx, IB_QPT_SMI, create_tun);
if (ret) {
pr_err("Couldn't create %s QP0 (%d)\n",
create_tun ? "tunnel for" : "", ret);
- goto err_mr;
+ goto err_pd;
}
}
@@ -1866,10 +1859,6 @@ err_qp0:
ib_destroy_qp(ctx->qp[0].qp);
ctx->qp[0].qp = NULL;
-err_mr:
- ib_dereg_mr(ctx->mr);
- ctx->mr = NULL;
-
err_pd:
ib_dealloc_pd(ctx->pd);
ctx->pd = NULL;
@@ -1906,8 +1895,6 @@ static void destroy_pv_resources(struct mlx4_ib_dev *dev, int slave, int port,
ib_destroy_qp(ctx->qp[1].qp);
ctx->qp[1].qp = NULL;
mlx4_ib_free_pv_qp_bufs(ctx, IB_QPT_GSI, 1);
- ib_dereg_mr(ctx->mr);
- ctx->mr = NULL;
ib_dealloc_pd(ctx->pd);
ctx->pd = NULL;
ib_destroy_cq(ctx->cq);
@@ -2040,8 +2027,6 @@ static void mlx4_ib_free_sqp_ctx(struct mlx4_ib_demux_pv_ctx *sqp_ctx)
ib_destroy_qp(sqp_ctx->qp[1].qp);
sqp_ctx->qp[1].qp = NULL;
mlx4_ib_free_pv_qp_bufs(sqp_ctx, IB_QPT_GSI, 0);
- ib_dereg_mr(sqp_ctx->mr);
- sqp_ctx->mr = NULL;
ib_dealloc_pd(sqp_ctx->pd);
sqp_ctx->pd = NULL;
ib_destroy_cq(sqp_ctx->cq);
diff --git a/drivers/infiniband/hw/mlx4/mlx4_ib.h b/drivers/infiniband/hw/mlx4/mlx4_ib.h
index 334387f63358..9066fc2406cc 100644
--- a/drivers/infiniband/hw/mlx4/mlx4_ib.h
+++ b/drivers/infiniband/hw/mlx4/mlx4_ib.h
@@ -415,7 +415,6 @@ struct mlx4_ib_demux_pv_ctx {
struct ib_device *ib_dev;
struct ib_cq *cq;
struct ib_pd *pd;
- struct ib_mr *mr;
struct work_struct work;
struct workqueue_struct *wq;
struct mlx4_ib_demux_pv_qp qp[2];
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/10] IB/mlx5: Remove ib_get_dma_mr calls
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (2 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 04/10] IB/mlx4: Remove ib_get_dma_mr calls Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-22 23:34 ` [PATCH 06/10] IB/iser: Use pd->local_dma_lkey Jason Gunthorpe
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/infiniband/hw/mlx5/main.c | 13 -------------
drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 -
drivers/infiniband/hw/mlx5/mr.c | 5 ++---
3 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 085c24b4b603..72ab0bdb415a 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1121,7 +1121,6 @@ static void destroy_umrc_res(struct mlx5_ib_dev *dev)
mlx5_ib_destroy_qp(dev->umrc.qp);
ib_destroy_cq(dev->umrc.cq);
- ib_dereg_mr(dev->umrc.mr);
ib_dealloc_pd(dev->umrc.pd);
}
@@ -1136,7 +1135,6 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
struct ib_pd *pd;
struct ib_cq *cq;
struct ib_qp *qp;
- struct ib_mr *mr;
struct ib_cq_init_attr cq_attr = {};
int ret;
@@ -1154,13 +1152,6 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
goto error_0;
}
- mr = ib_get_dma_mr(pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(mr)) {
- mlx5_ib_dbg(dev, "Couldn't create DMA MR for sync UMR QP\n");
- ret = PTR_ERR(mr);
- goto error_1;
- }
-
cq_attr.cqe = 128;
cq = ib_create_cq(&dev->ib_dev, mlx5_umr_cq_handler, NULL, NULL,
&cq_attr);
@@ -1218,7 +1209,6 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
dev->umrc.qp = qp;
dev->umrc.cq = cq;
- dev->umrc.mr = mr;
dev->umrc.pd = pd;
sema_init(&dev->umrc.sem, MAX_UMR_WR);
@@ -1240,9 +1230,6 @@ error_3:
ib_destroy_cq(cq);
error_2:
- ib_dereg_mr(mr);
-
-error_1:
ib_dealloc_pd(pd);
error_0:
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 7cae09836481..446d80427773 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -349,7 +349,6 @@ struct umr_common {
struct ib_pd *pd;
struct ib_cq *cq;
struct ib_qp *qp;
- struct ib_mr *mr;
/* control access to UMR QP
*/
struct semaphore sem;
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index bc9a0de897cb..4c92ca8a4181 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -690,12 +690,11 @@ static void prep_umr_reg_wqe(struct ib_pd *pd, struct ib_send_wr *wr,
int access_flags)
{
struct mlx5_ib_dev *dev = to_mdev(pd->device);
- struct ib_mr *mr = dev->umrc.mr;
struct mlx5_umr_wr *umrwr = (struct mlx5_umr_wr *)&wr->wr.fast_reg;
sg->addr = dma;
sg->length = ALIGN(sizeof(u64) * n, 64);
- sg->lkey = mr->lkey;
+ sg->lkey = dev->umrc.pd->local_dma_lkey;
wr->next = NULL;
wr->send_flags = 0;
@@ -926,7 +925,7 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
sg.addr = dma;
sg.length = ALIGN(npages * sizeof(u64),
MLX5_UMR_MTT_ALIGNMENT);
- sg.lkey = dev->umrc.mr->lkey;
+ sg.lkey = dev->umrc.pd->local_dma_lkey;
wr.send_flags = MLX5_IB_SEND_UMR_FAIL_IF_FREE |
MLX5_IB_SEND_UMR_UPDATE_MTT;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 06/10] IB/iser: Use pd->local_dma_lkey
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (3 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 05/10] IB/mlx5: " Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-23 10:49 ` Sagi Grimberg
2015-07-22 23:34 ` [PATCH 09/10] ib_srpt: Remove ib_get_dma_mr calls Jason Gunthorpe
` (4 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
Replace all leys with pd->local_dma_lkey. This driver does not support
iWarp, so this is safe.
The insecure use of ib_get_dma_mr is thus isolated to an rkey, and this
looks trivially fixed by forcing the use of registration in a future
patch.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/infiniband/ulp/iser/iscsi_iser.c | 2 +-
drivers/infiniband/ulp/iser/iser_initiator.c | 8 ++++----
drivers/infiniband/ulp/iser/iser_memory.c | 2 +-
drivers/infiniband/ulp/iser/iser_verbs.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 6a594aac2290..f44c6b879329 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -204,7 +204,7 @@ iser_initialize_task_headers(struct iscsi_task *task,
tx_desc->dma_addr = dma_addr;
tx_desc->tx_sg[0].addr = tx_desc->dma_addr;
tx_desc->tx_sg[0].length = ISER_HEADERS_LEN;
- tx_desc->tx_sg[0].lkey = device->mr->lkey;
+ tx_desc->tx_sg[0].lkey = device->pd->local_dma_lkey;
iser_task->iser_conn = iser_conn;
out:
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 3e2118e8ed87..2d02f042c69a 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -173,8 +173,8 @@ static void iser_create_send_desc(struct iser_conn *iser_conn,
tx_desc->num_sge = 1;
- if (tx_desc->tx_sg[0].lkey != device->mr->lkey) {
- tx_desc->tx_sg[0].lkey = device->mr->lkey;
+ if (tx_desc->tx_sg[0].lkey != device->pd->local_dma_lkey) {
+ tx_desc->tx_sg[0].lkey = device->pd->local_dma_lkey;
iser_dbg("sdesc %p lkey mismatch, fixing\n", tx_desc);
}
}
@@ -291,7 +291,7 @@ int iser_alloc_rx_descriptors(struct iser_conn *iser_conn,
rx_sg = &rx_desc->rx_sg;
rx_sg->addr = rx_desc->dma_addr;
rx_sg->length = ISER_RX_PAYLOAD_SIZE;
- rx_sg->lkey = device->mr->lkey;
+ rx_sg->lkey = device->pd->local_dma_lkey;
}
iser_conn->rx_desc_head = 0;
@@ -543,7 +543,7 @@ int iser_send_control(struct iscsi_conn *conn,
tx_dsg->addr = iser_conn->login_req_dma;
tx_dsg->length = task->data_count;
- tx_dsg->lkey = device->mr->lkey;
+ tx_dsg->lkey = device->pd->local_dma_lkey;
mdesc->num_sge = 2;
}
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index f0cdc961eb11..3129a42150ff 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -393,7 +393,7 @@ iser_reg_dma(struct iser_device *device, struct iser_data_buf *mem,
{
struct scatterlist *sg = mem->sg;
- reg->sge.lkey = device->mr->lkey;
+ reg->sge.lkey = device->pd->local_dma_lkey;
reg->rkey = device->mr->rkey;
reg->sge.addr = ib_sg_dma_address(device->ib_device, &sg[0]);
reg->sge.length = ib_sg_dma_len(device->ib_device, &sg[0]);
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 5c9f565ea0e8..52268356c79e 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -1017,7 +1017,7 @@ int iser_post_recvl(struct iser_conn *iser_conn)
sge.addr = iser_conn->login_resp_dma;
sge.length = ISER_RX_LOGIN_SIZE;
- sge.lkey = ib_conn->device->mr->lkey;
+ sge.lkey = ib_conn->device->pd->local_dma_lkey;
rx_wr.wr_id = (uintptr_t)iser_conn->login_resp_buf;
rx_wr.sg_list = &sge;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 06/10] IB/iser: Use pd->local_dma_lkey
2015-07-22 23:34 ` [PATCH 06/10] IB/iser: Use pd->local_dma_lkey Jason Gunthorpe
@ 2015-07-23 10:49 ` Sagi Grimberg
0 siblings, 0 replies; 31+ messages in thread
From: Sagi Grimberg @ 2015-07-23 10:49 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
On 7/23/2015 2:34 AM, Jason Gunthorpe wrote:
> Replace all leys with pd->local_dma_lkey. This driver does not support
> iWarp, so this is safe.
>
> The insecure use of ib_get_dma_mr is thus isolated to an rkey, and this
> looks trivially fixed by forcing the use of registration in a future
> patch.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Looks good.
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/10] ib_srpt: Remove ib_get_dma_mr calls
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (4 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 06/10] IB/iser: Use pd->local_dma_lkey Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-23 10:51 ` Sagi Grimberg
2015-07-22 23:34 ` [PATCH 10/10] net/9p: " Jason Gunthorpe
` (3 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 15 ++++-----------
drivers/infiniband/ulp/srpt/ib_srpt.h | 1 -
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 82897ca17f32..07da3817d92d 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -783,7 +783,7 @@ static int srpt_post_recv(struct srpt_device *sdev,
list.addr = ioctx->ioctx.dma;
list.length = srp_max_req_size;
- list.lkey = sdev->mr->lkey;
+ list.lkey = sdev->pd->local_dma_lkey;
wr.next = NULL;
wr.sg_list = &list;
@@ -818,7 +818,7 @@ static int srpt_post_send(struct srpt_rdma_ch *ch,
list.addr = ioctx->ioctx.dma;
list.length = len;
- list.lkey = sdev->mr->lkey;
+ list.lkey = sdev->pd->local_dma_lkey;
wr.next = NULL;
wr.wr_id = encode_wr_id(SRPT_SEND, ioctx->ioctx.index);
@@ -1206,7 +1206,7 @@ static int srpt_map_sg_to_ib_sge(struct srpt_rdma_ch *ch,
while (rsize > 0 && tsize > 0) {
sge->addr = dma_addr;
- sge->lkey = ch->sport->sdev->mr->lkey;
+ sge->lkey = ch->sport->sdev->pd->local_dma_lkey;
if (rsize >= dma_len) {
sge->length =
@@ -3212,10 +3212,6 @@ static void srpt_add_one(struct ib_device *device)
if (IS_ERR(sdev->pd))
goto free_dev;
- sdev->mr = ib_get_dma_mr(sdev->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(sdev->mr))
- goto err_pd;
-
sdev->srq_size = min(srpt_srq_size, sdev->dev_attr.max_srq_wr);
srq_attr.event_handler = srpt_srq_event;
@@ -3227,7 +3223,7 @@ static void srpt_add_one(struct ib_device *device)
sdev->srq = ib_create_srq(sdev->pd, &srq_attr);
if (IS_ERR(sdev->srq))
- goto err_mr;
+ goto err_pd;
pr_debug("%s: create SRQ #wr= %d max_allow=%d dev= %s\n",
__func__, sdev->srq_size, sdev->dev_attr.max_srq_wr,
@@ -3312,8 +3308,6 @@ err_cm:
ib_destroy_cm_id(sdev->cm_id);
err_srq:
ib_destroy_srq(sdev->srq);
-err_mr:
- ib_dereg_mr(sdev->mr);
err_pd:
ib_dealloc_pd(sdev->pd);
free_dev:
@@ -3359,7 +3353,6 @@ static void srpt_remove_one(struct ib_device *device)
srpt_release_sdev(sdev);
ib_destroy_srq(sdev->srq);
- ib_dereg_mr(sdev->mr);
ib_dealloc_pd(sdev->pd);
srpt_free_ioctx_ring((struct srpt_ioctx **)sdev->ioctx_ring, sdev,
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 21f8df67522a..5faad8acd789 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -393,7 +393,6 @@ struct srpt_port {
struct srpt_device {
struct ib_device *device;
struct ib_pd *pd;
- struct ib_mr *mr;
struct ib_srq *srq;
struct ib_cm_id *cm_id;
struct ib_device_attr dev_attr;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 09/10] ib_srpt: Remove ib_get_dma_mr calls
2015-07-22 23:34 ` [PATCH 09/10] ib_srpt: Remove ib_get_dma_mr calls Jason Gunthorpe
@ 2015-07-23 10:51 ` Sagi Grimberg
0 siblings, 0 replies; 31+ messages in thread
From: Sagi Grimberg @ 2015-07-23 10:51 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
On 7/23/2015 2:34 AM, Jason Gunthorpe wrote:
> The pd now has a local_dma_lkey member which completely replaces
> ib_get_dma_mr, use it instead.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Looks good.
Reviewed-by: Sagi Grimberg <sagig@mellanox.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/10] net/9p: Remove ib_get_dma_mr calls
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (5 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 09/10] ib_srpt: Remove ib_get_dma_mr calls Jason Gunthorpe
@ 2015-07-22 23:34 ` Jason Gunthorpe
2015-07-23 7:46 ` Dominique Martinet
2015-07-23 10:56 ` [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Sagi Grimberg
` (2 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-22 23:34 UTC (permalink / raw)
To: Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
The pd now has a local_dma_lkey member which completely replaces
ib_get_dma_mr, use it instead.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
net/9p/trans_rdma.c | 26 ++------------------------
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
index 37a78d20c0f6..ba1210253f5e 100644
--- a/net/9p/trans_rdma.c
+++ b/net/9p/trans_rdma.c
@@ -94,8 +94,6 @@ struct p9_trans_rdma {
struct ib_pd *pd;
struct ib_qp *qp;
struct ib_cq *cq;
- struct ib_mr *dma_mr;
- u32 lkey;
long timeout;
int sq_depth;
struct semaphore sq_sem;
@@ -382,9 +380,6 @@ static void rdma_destroy_trans(struct p9_trans_rdma *rdma)
if (!rdma)
return;
- if (rdma->dma_mr && !IS_ERR(rdma->dma_mr))
- ib_dereg_mr(rdma->dma_mr);
-
if (rdma->qp && !IS_ERR(rdma->qp))
ib_destroy_qp(rdma->qp);
@@ -415,7 +410,7 @@ post_recv(struct p9_client *client, struct p9_rdma_context *c)
sge.addr = c->busa;
sge.length = client->msize;
- sge.lkey = rdma->lkey;
+ sge.lkey = rdma->pd->local_dma_lkey;
wr.next = NULL;
c->wc_op = IB_WC_RECV;
@@ -506,7 +501,7 @@ dont_need_post_recv:
sge.addr = c->busa;
sge.length = c->req->tc->size;
- sge.lkey = rdma->lkey;
+ sge.lkey = rdma->pd->local_dma_lkey;
wr.next = NULL;
c->wc_op = IB_WC_SEND;
@@ -647,7 +642,6 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args)
struct p9_trans_rdma *rdma;
struct rdma_conn_param conn_param;
struct ib_qp_init_attr qp_attr;
- struct ib_device_attr devattr;
struct ib_cq_init_attr cq_attr = {};
/* Parse the transport specific mount options */
@@ -700,11 +694,6 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args)
if (err || (rdma->state != P9_RDMA_ROUTE_RESOLVED))
goto error;
- /* Query the device attributes */
- err = ib_query_device(rdma->cm_id->device, &devattr);
- if (err)
- goto error;
-
/* Create the Completion Queue */
cq_attr.cqe = opts.sq_depth + opts.rq_depth + 1;
rdma->cq = ib_create_cq(rdma->cm_id->device, cq_comp_handler,
@@ -719,17 +708,6 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args)
if (IS_ERR(rdma->pd))
goto error;
- /* Cache the DMA lkey in the transport */
- rdma->dma_mr = NULL;
- if (devattr.device_cap_flags & IB_DEVICE_LOCAL_DMA_LKEY)
- rdma->lkey = rdma->cm_id->device->local_dma_lkey;
- else {
- rdma->dma_mr = ib_get_dma_mr(rdma->pd, IB_ACCESS_LOCAL_WRITE);
- if (IS_ERR(rdma->dma_mr))
- goto error;
- rdma->lkey = rdma->dma_mr->lkey;
- }
-
/* Create the Queue Pair */
memset(&qp_attr, 0, sizeof qp_attr);
qp_attr.event_handler = qp_event_handler;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 10/10] net/9p: Remove ib_get_dma_mr calls
2015-07-22 23:34 ` [PATCH 10/10] net/9p: " Jason Gunthorpe
@ 2015-07-23 7:46 ` Dominique Martinet
0 siblings, 0 replies; 31+ messages in thread
From: Dominique Martinet @ 2015-07-23 7:46 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma, Amir Vadai, Andy Grover,
Bart Van Assche, Chien Yen, Christoph Hellwig, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
Jason Gunthorpe wrote on Wed, Jul 22, 2015 at 05:34:43PM -0600:
> The pd now has a local_dma_lkey member which completely replaces
> ib_get_dma_mr, use it instead.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
9P part looks good and tested OK (only applied 01 and 10 of the serie),
thanks for the Cc.
--
Dominique
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (6 preceding siblings ...)
2015-07-22 23:34 ` [PATCH 10/10] net/9p: " Jason Gunthorpe
@ 2015-07-23 10:56 ` Sagi Grimberg
2015-07-23 13:47 ` Bart Van Assche
2015-07-28 15:01 ` J.L. Burr
9 siblings, 0 replies; 31+ messages in thread
From: Sagi Grimberg @ 2015-07-23 10:56 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Bart Van Assche, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
> Sagi, IB/iser should have special attention paid, as it is less clear to me if
> it got everything.
It looks fine. I'll pull those as well.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (7 preceding siblings ...)
2015-07-23 10:56 ` [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Sagi Grimberg
@ 2015-07-23 13:47 ` Bart Van Assche
2015-07-23 18:30 ` Jason Gunthorpe
2015-07-25 6:27 ` Christoph Hellwig
2015-07-28 15:01 ` J.L. Burr
9 siblings, 2 replies; 31+ messages in thread
From: Bart Van Assche @ 2015-07-23 13:47 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford, linux-rdma
Cc: Amir Vadai, Andy Grover, Chien Yen, Christoph Hellwig,
Dominique Martinet, Eli Cohen, Eric Van Hensbergen, Ido Shamay,
Latchesar Ionkov, Or Gerlitz, Roi Dayan, Ron Minnich,
Sagi Grimberg, Simon Derr, Tom Tucker, Zach Brown, rds-devel,
target-devel, v9fs-developer
On 07/22/15 16:34, Jason Gunthorpe wrote:
> The remaining users of ib_get_dma_mr are all unsafe:
> [ ... ]
> drivers/infiniband/ulp/srp/ib_srp.c:
> srp_dev->mr = ib_get_dma_mr(srp_dev->pd,
> IB_ACCESS_LOCAL_WRITE |
> IB_ACCESS_REMOTE_READ |
> IB_ACCESS_REMOTE_WRITE);
Hello Jason,
This statement might need some clarification. Are you aware that this
memory region is only used if the kernel module parameter
register_always is zero ?
I will try to find some time to test the SRP changes in this series but
I'm not sure yet when I will be able to do that.
Bart.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-23 13:47 ` Bart Van Assche
@ 2015-07-23 18:30 ` Jason Gunthorpe
[not found] ` <20150723183044.GA1868-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-07-25 6:27 ` Christoph Hellwig
1 sibling, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-23 18:30 UTC (permalink / raw)
To: Bart Van Assche
Cc: Doug Ledford, linux-rdma, Amir Vadai, Andy Grover, Chien Yen,
Christoph Hellwig, Dominique Martinet, Eli Cohen,
Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov, Or Gerlitz,
Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr, Tom Tucker,
Zach Brown, rds-devel, target-devel, v9fs-developer
On Thu, Jul 23, 2015 at 06:47:26AM -0700, Bart Van Assche wrote:
> On 07/22/15 16:34, Jason Gunthorpe wrote:
> >The remaining users of ib_get_dma_mr are all unsafe:
> > [ ... ]
> > drivers/infiniband/ulp/srp/ib_srp.c:
> > srp_dev->mr = ib_get_dma_mr(srp_dev->pd,
> > IB_ACCESS_LOCAL_WRITE |
> > IB_ACCESS_REMOTE_READ |
> > IB_ACCESS_REMOTE_WRITE);
>
> Hello Jason,
>
> This statement might need some clarification. Are you aware that this memory
> region is only used if the kernel module parameter register_always is zero ?
It doesn't matter, just making the above call is enough to create the
security problem, even if the rkey is not used.
I looked at the register_always stuff, it isn't obvious to me that it
interacts with this path:
static int srp_map_sg(struct srp_map_state *state, struct srp_rdma_ch *ch,
if (dev->use_fast_reg) {
} else {
use_mr = !!ch->fmr_pool;
[..]
if (srp_map_sg_entry(state, ch, sg, i, use_mr)) {
static int srp_map_sg_entry(struct srp_map_state *state,
if (!use_mr) {
srp_map_desc(state, dma_addr, dma_len, target->rkey);
Perhaps the above is dead code, all our IB drivers either support FMR
or FRWR, so maybe use_mr is always true?
It looks to me like register_always is similar to iSER, it is trying
to avoid a MR if there is only 1 S/G entry. That performance behavior
needs to default to disabled. The kernel must default to secure out of
the box.
> I will try to find some time to test the SRP changes in this series but I'm
> not sure yet when I will be able to do that.
This probably also takes care of the security issue for SRP, what do you
think?
>From d1ae6713fc011b6ff392e42cfb342899da8561ff Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Thu, 23 Jul 2015 12:19:53 -0600
Subject: [PATCH] IB/srp: Do not create an all physical insecure rkey by
default
The ULP only needs this if the insecure register_always performance
optimization is enabled, or if FRWR/FMR is not supported in the driver.
Do not create an all physical MR unless it is needed to support either
of those modes. Default register_always to true so the out of the box
configuration does not create an insecure all physical MR.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/infiniband/ulp/srp/ib_srp.c | 31 +++++++++++++++++++++----------
drivers/infiniband/ulp/srp/ib_srp.h | 2 +-
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index fb9fed0fac28..a1e3818d0791 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -69,7 +69,7 @@ static unsigned int cmd_sg_entries;
static unsigned int indirect_sg_entries;
static bool allow_ext_sg;
static bool prefer_fr;
-static bool register_always;
+static bool register_always = true;
static int topspin_workarounds = 1;
module_param(srp_sg_tablesize, uint, 0444);
@@ -3150,7 +3150,8 @@ static ssize_t srp_create_target(struct device *dev,
target->scsi_host = target_host;
target->srp_host = host;
target->lkey = host->srp_dev->pd->local_dma_lkey;
- target->rkey = host->srp_dev->mr->rkey;
+ if (host->srp_dev->rkey_mr)
+ target->rkey = host->srp_dev->rkey_mr->rkey;
target->cmd_sg_cnt = cmd_sg_entries;
target->sg_tablesize = indirect_sg_entries ? : cmd_sg_entries;
target->allow_ext_sg = allow_ext_sg;
@@ -3381,6 +3382,7 @@ static void srp_add_one(struct ib_device *device)
struct srp_host *host;
int mr_page_shift, s, e, p;
u64 max_pages_per_mr;
+ unsigned int mr_flags = 0;
dev_attr = kmalloc(sizeof *dev_attr, GFP_KERNEL);
if (!dev_attr)
@@ -3399,8 +3401,11 @@ static void srp_add_one(struct ib_device *device)
device->map_phys_fmr && device->unmap_fmr);
srp_dev->has_fr = (dev_attr->device_cap_flags &
IB_DEVICE_MEM_MGT_EXTENSIONS);
- if (!srp_dev->has_fmr && !srp_dev->has_fr)
+ if (!srp_dev->has_fmr && !srp_dev->has_fr) {
dev_warn(&device->dev, "neither FMR nor FR is supported\n");
+ /* Fall back to using an insecure all physical rkey */
+ mr_flags |= IB_ACCESS_REMOTE_READ | IB_ACCESS_REMOTE_WRITE;
+ }
srp_dev->use_fast_reg = (srp_dev->has_fr &&
(!srp_dev->has_fmr || prefer_fr));
@@ -3436,12 +3441,17 @@ static void srp_add_one(struct ib_device *device)
if (IS_ERR(srp_dev->pd))
goto free_dev;
- srp_dev->mr = ib_get_dma_mr(srp_dev->pd,
- IB_ACCESS_LOCAL_WRITE |
- IB_ACCESS_REMOTE_READ |
- IB_ACCESS_REMOTE_WRITE);
- if (IS_ERR(srp_dev->mr))
- goto err_pd;
+ if (register_always)
+ mr_flags |= IB_ACCESS_REMOTE_READ | IB_ACCESS_REMOTE_WRITE;
+
+ if (mr_flags) {
+ srp_dev->rkey_mr = ib_get_dma_mr(
+ srp_dev->pd, IB_ACCESS_LOCAL_WRITE | mr_flags);
+ if (IS_ERR(srp_dev->rkey_mr))
+ goto err_pd;
+ } else
+ srp_dev->rkey_mr = NULL;
+
if (device->node_type == RDMA_NODE_IB_SWITCH) {
s = 0;
@@ -3506,7 +3516,8 @@ static void srp_remove_one(struct ib_device *device)
kfree(host);
}
- ib_dereg_mr(srp_dev->mr);
+ if (srp_dev->rkey_mr)
+ ib_dereg_mr(srp_dev->rkey_mr);
ib_dealloc_pd(srp_dev->pd);
kfree(srp_dev);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 17ee3f80ba55..8b241f17f8b8 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -95,7 +95,7 @@ struct srp_device {
struct list_head dev_list;
struct ib_device *dev;
struct ib_pd *pd;
- struct ib_mr *mr;
+ struct ib_mr *rkey_mr;
u64 mr_page_mask;
int mr_page_size;
int mr_max_size;
--
2.1.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-23 13:47 ` Bart Van Assche
2015-07-23 18:30 ` Jason Gunthorpe
@ 2015-07-25 6:27 ` Christoph Hellwig
1 sibling, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2015-07-25 6:27 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jason Gunthorpe, Doug Ledford, linux-rdma, Amir Vadai,
Andy Grover, Chien Yen, Christoph Hellwig, Dominique Martinet,
Eli Cohen, Eric Van Hensbergen, Ido Shamay, Latchesar Ionkov,
Or Gerlitz, Roi Dayan, Ron Minnich, Sagi Grimberg, Simon Derr,
Tom Tucker, Zach Brown, rds-devel, target-devel, v9fs-developer
Hi Bart,
On Thu, Jul 23, 2015 at 06:47:26AM -0700, Bart Van Assche wrote:
> This statement might need some clarification. Are you aware that this memory
> region is only used if the kernel module parameter register_always is zero ?
The way I read the driver it's also used if the driver doesn't support
either FMR or FR (although no current in-tree IB driver falls into that
category), or as fallback if memory registration fails for some reason.
But given that rkeys are just 32-bit integers there also seems to be
some risk that a peer could simply brute force it even if it's never
sent out on the wire.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-22 23:34 [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey Jason Gunthorpe
` (8 preceding siblings ...)
2015-07-23 13:47 ` Bart Van Assche
@ 2015-07-28 15:01 ` J.L. Burr
2015-07-28 18:23 ` Jason Gunthorpe
9 siblings, 1 reply; 31+ messages in thread
From: J.L. Burr @ 2015-07-28 15:01 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
"Jason Gunthorpe" <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote in message
news:1437608083-22898-1-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMK2raWmgRF17@public.gmane.org
> This series moves dealing with the safe all physical mr:
>
> ib_get_dma_mr(pd,IB_ACCESS_LOCAL_WRITE);
>
> Into ib_alloc_pd, and in the process makes the global local_dma_lkey functionality
> broadly enabled for all ULPs.
> The remaining users of ib_get_dma_mr are all unsafe:
[snip]
> Calling ib_get_dma_mr with IB_ACCESS_REMOTE_* flags is considered to be a
> serious security problem and should not be done without the user directly
> opting in to an off-by-default scheme. The call allows the peer on the QP
> unrestricted access to local physical memory if they can guess the rkey value.
>
> A future series will cause the kernel to be tainted by the above call sites to
> promote migrating away from this.
[snip]
We use ib_get_dma_mr with IB_ACCESS_REMOTE_* flags in an embedded device environment (in a custom out-of-tree device driver). Not
to allow remote access to CPU memory but to allow remote access to PCIe device memory (the IB card makes peer accesses directly to
other PCIe devices).
I understand the concerns about safety with ib_get_dma_mr but want to be sure there will be a way to "opt-in", as you describe
above. I can live with the tainting.
How is the opt-in done? Is that via a kernel parameter? Or via a module parameter on one of the IB core modules? Some other way?
I'm trying to avoid having to run a modified kernel for our environment, so hope this is not a kernel compile option!
Thanks.
John Burr
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-28 15:01 ` J.L. Burr
@ 2015-07-28 18:23 ` Jason Gunthorpe
2015-07-28 20:58 ` J.L. Burr
0 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-28 18:23 UTC (permalink / raw)
To: J.L. Burr; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
On Tue, Jul 28, 2015 at 11:01:12AM -0400, J.L. Burr wrote:
> We use ib_get_dma_mr with IB_ACCESS_REMOTE_* flags in an embedded
> device environment (in a custom out-of-tree device driver). Not to
> allow remote access to CPU memory but to allow remote access to PCIe
> device memory (the IB card makes peer accesses directly to other
> PCIe devices).
Why can't you create a proper MR that only exposes the PCI device's
BAR?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-28 18:23 ` Jason Gunthorpe
@ 2015-07-28 20:58 ` J.L. Burr
2015-07-28 22:10 ` Jason Gunthorpe
0 siblings, 1 reply; 31+ messages in thread
From: J.L. Burr @ 2015-07-28 20:58 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
"Jason Gunthorpe"
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote in
message news:20150728182356.GA1712-ePGOBjL8dl3ta4EC/59zMK2raWmgRF17@public.gmane.org
> On Tue, Jul 28, 2015 at 11:01:12AM -0400, J.L. Burr wrote:
>
>> We use ib_get_dma_mr with IB_ACCESS_REMOTE_* flags in an embedded
>> device environment (in a custom out-of-tree device driver). Not to
>> allow remote access to CPU memory but to allow remote access to PCIe
>> device memory (the IB card makes peer accesses directly to other
>> PCIe devices).
>
> Why can't you create a proper MR that only exposes the PCI device's
> BAR?
On the embedded side, the QPs and MRs are all in kernel space. That's
because the PCIe device BARs are huge (2**39 is the typical size, but it
can be as large as 2**47). This is much too large to map into the
embedded processor's address space.
In an earlier implementation, I modified ib_reg_phys_mr so that it would
return a no-translation MR and could also specify the physical address
range I wanted it to define. But I haven't used that in a long time.
Plus you guys are removing ib_reg_phys_mr anyway! :-)
My current scheme, with ib_get_dma_mr, results in a MR which maps the
entire 64-bit physical space (which isn't ideal; would indeed be better
if the MR was limited to a single PCIe device BAR space) but does have
the advantage (to me) of not requiring any modifications to Linux, the
kernel IB stack, or IB hardware drivers.
I'm admittedly (way) out of date. Our embedded system is running CentOS
6.6 so the kernel level (2.6.32) is ancient by upstream standards.
Is there some way now (in upstream kernels) to create a MR with an
arbitrary (and large) physical address range? That would be great! I
didn't see a way to do that when I started on this journey (about 4
years ago).
Thanks.
John
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-28 20:58 ` J.L. Burr
@ 2015-07-28 22:10 ` Jason Gunthorpe
2015-07-28 23:56 ` J.L. Burr
0 siblings, 1 reply; 31+ messages in thread
From: Jason Gunthorpe @ 2015-07-28 22:10 UTC (permalink / raw)
To: J.L. Burr; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
On Tue, Jul 28, 2015 at 04:58:29PM -0400, J.L. Burr wrote:
> Is there some way now (in upstream kernels) to create a MR with an
> arbitrary (and large) physical address range? That would be great! I
> didn't see a way to do that when I started on this journey (about 4
> years ago).
The FRWR API can do this, but it depends on each card if it can manage
the page list table or not.
You'd need to check page_size_cap and max_map_per_fmr for your card to
see.
Basically use an array of the largest page size your adaptor will
support.
Eg mlx5 supports any page size above 4k, so you can do any PCI BAR
with a single FRWR page table entry.
mlx4 seems to top out at 2G pages (which may just be the driver being
silly) so you'd need 64k page entries, no idea if it can do that or
not..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey
2015-07-28 22:10 ` Jason Gunthorpe
@ 2015-07-28 23:56 ` J.L. Burr
0 siblings, 0 replies; 31+ messages in thread
From: J.L. Burr @ 2015-07-28 23:56 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
"Jason Gunthorpe"
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote in
message news:20150728221050.GA1279-ePGOBjL8dl3ta4EC/59zMK2raWmgRF17@public.gmane.org
> On Tue, Jul 28, 2015 at 04:58:29PM -0400, J.L. Burr wrote:
>
>> Is there some way now (in upstream kernels) to create a MR with an
>> arbitrary (and large) physical address range? That would be great!
>> I
>> didn't see a way to do that when I started on this journey (about 4
>> years ago).
>
> The FRWR API can do this, but it depends on each card if it can manage
> the page list table or not.
>
> You'd need to check page_size_cap and max_map_per_fmr for your card to
> see.
>
> Basically use an array of the largest page size your adaptor will
> support.
>
> Eg mlx5 supports any page size above 4k, so you can do any PCI BAR
> with a single FRWR page table entry.
>
> mlx4 seems to top out at 2G pages (which may just be the driver being
> silly) so you'd need 64k page entries, no idea if it can do that or
> not..
Well, the beauty of the scheme I'm using now is there are no paging
aspects at all, just a flat 64-bit *physical* address space usable with
the MR returned from ib_get_dma_mr.
Again, all of the targeted memory spaces are in PCIe BAR space on
physical devices. So, no paging, page tables, or pinning, etc. It's
always there (well, hardware-willing)! As the local CPU cannot access
this space (too big to map), it can only be accessed remotely via RDMA
requests.
I realize this is a bit unusual. So, that's why I'm OK for tainting as
long as I have a way to opt-in to maintain the current ib_get_dma_mr
scheme. I understand that this would not be desirable for a normal
environment.
John
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread