* [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions
@ 2022-06-16 14:09 Md Haris Iqbal
2022-06-22 14:12 ` haris iqbal
0 siblings, 1 reply; 4+ messages in thread
From: Md Haris Iqbal @ 2022-06-16 14:09 UTC (permalink / raw)
To: linux-rdma
Cc: leon, jgg, zyjzyj2000, haris.iqbal, jinpu.wang, aleksei.marov,
Md Haris Iqbal, rpearsonhpe
Currently rxe_invalidate_mr does invalidate for both local ops, and remote
ones. This means that MR being invalidated is compared with rkey for both,
which is incorrect. For local invalidate, comparison should happen with
lkey, and for the remote one, it should happen with rkey.
This commit splits the rxe_invalidate_mr into local and remote versions.
Each of them does comparison the right way as described above (with lkey
for local, and rkey for remote).
Fixes: 3902b429ca14 ("RDMA/rxe: Implement invalidate MW operations")
Cc: rpearsonhpe@gmail.com
Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com>
---
v1 -> v2
give a better name to key variable in function rxe_do_local_ops
drivers/infiniband/sw/rxe/rxe_loc.h | 3 +-
drivers/infiniband/sw/rxe/rxe_mr.c | 59 +++++++++++++++++++++-------
drivers/infiniband/sw/rxe/rxe_req.c | 10 ++---
drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
4 files changed, 53 insertions(+), 21 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 0e022ae1b8a5..4da57abbbc8c 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -77,7 +77,8 @@ struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
enum rxe_mr_lookup_type type);
int mr_check_range(struct rxe_mr *mr, u64 iova, size_t length);
int advance_dma_data(struct rxe_dma_info *dma, unsigned int length);
-int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey);
+int rxe_invalidate_mr_local(struct rxe_qp *qp, u32 lkey);
+int rxe_invalidate_mr_remote(struct rxe_qp *qp, u32 rkey);
int rxe_reg_fast_mr(struct rxe_qp *qp, struct rxe_send_wqe *wqe);
int rxe_mr_set_page(struct ib_mr *ibmr, u64 addr);
int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata);
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index fc3942e04a1f..1c7179dd92eb 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -576,41 +576,72 @@ struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
return mr;
}
-int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey)
+static int rxe_invalidate_mr(struct rxe_mr *mr)
+{
+ if (atomic_read(&mr->num_mw) > 0) {
+ pr_warn("%s: Attempt to invalidate an MR while bound to MWs\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ if (unlikely(mr->type != IB_MR_TYPE_MEM_REG)) {
+ pr_warn("%s: mr->type (%d) is wrong type\n", __func__, mr->type);
+ return -EINVAL;
+ }
+
+ mr->state = RXE_MR_STATE_FREE;
+ return 0;
+}
+
+int rxe_invalidate_mr_local(struct rxe_qp *qp, u32 lkey)
{
struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
struct rxe_mr *mr;
int ret;
- mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
+ mr = rxe_pool_get_index(&rxe->mr_pool, lkey >> 8);
if (!mr) {
- pr_err("%s: No MR for rkey %#x\n", __func__, rkey);
+ pr_err("%s: No MR for lkey %#x\n", __func__, lkey);
ret = -EINVAL;
goto err;
}
- if (rkey != mr->rkey) {
- pr_err("%s: rkey (%#x) doesn't match mr->rkey (%#x)\n",
- __func__, rkey, mr->rkey);
+ if (lkey != mr->lkey) {
+ pr_err("%s: lkey (%#x) doesn't match mr->lkey (%#x)\n",
+ __func__, lkey, mr->lkey);
ret = -EINVAL;
goto err_drop_ref;
}
- if (atomic_read(&mr->num_mw) > 0) {
- pr_warn("%s: Attempt to invalidate an MR while bound to MWs\n",
- __func__);
+ ret = rxe_invalidate_mr(mr);
+
+err_drop_ref:
+ rxe_put(mr);
+err:
+ return ret;
+}
+
+int rxe_invalidate_mr_remote(struct rxe_qp *qp, u32 rkey)
+{
+ struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
+ struct rxe_mr *mr;
+ int ret;
+
+ mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
+ if (!mr) {
+ pr_err("%s: No MR for rkey %#x\n", __func__, rkey);
ret = -EINVAL;
- goto err_drop_ref;
+ goto err;
}
- if (unlikely(mr->type != IB_MR_TYPE_MEM_REG)) {
- pr_warn("%s: mr->type (%d) is wrong type\n", __func__, mr->type);
+ if (rkey != mr->rkey) {
+ pr_err("%s: rkey (%#x) doesn't match mr->rkey (%#x)\n",
+ __func__, rkey, mr->rkey);
ret = -EINVAL;
goto err_drop_ref;
}
- mr->state = RXE_MR_STATE_FREE;
- ret = 0;
+ ret = rxe_invalidate_mr(mr);
err_drop_ref:
rxe_put(mr);
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 9d98237389cf..ef193a8a7158 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -541,16 +541,16 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
{
u8 opcode = wqe->wr.opcode;
- u32 rkey;
+ u32 key;
int ret;
switch (opcode) {
case IB_WR_LOCAL_INV:
- rkey = wqe->wr.ex.invalidate_rkey;
- if (rkey_is_mw(rkey))
- ret = rxe_invalidate_mw(qp, rkey);
+ key = wqe->wr.ex.invalidate_rkey;
+ if (rkey_is_mw(key))
+ ret = rxe_invalidate_mw(qp, key);
else
- ret = rxe_invalidate_mr(qp, rkey);
+ ret = rxe_invalidate_mr_local(qp, key);
if (unlikely(ret)) {
wqe->status = IB_WC_LOC_QP_OP_ERR;
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index f4f6ee5d81fe..01411280cd73 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -818,7 +818,7 @@ static int invalidate_rkey(struct rxe_qp *qp, u32 rkey)
if (rkey_is_mw(rkey))
return rxe_invalidate_mw(qp, rkey);
else
- return rxe_invalidate_mr(qp, rkey);
+ return rxe_invalidate_mr_remote(qp, rkey);
}
/* Executes a new request. A retried request never reach that function (send
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions
2022-06-16 14:09 [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions Md Haris Iqbal
@ 2022-06-22 14:12 ` haris iqbal
2022-06-24 20:02 ` Jason Gunthorpe
0 siblings, 1 reply; 4+ messages in thread
From: haris iqbal @ 2022-06-22 14:12 UTC (permalink / raw)
To: linux-rdma
Cc: Leon Romanovsky, jgg, zyjzyj2000, haris.iqbal, Jinpu Wang,
aleksei.marov, rpearsonhpe
On Thu, Jun 16, 2022 at 4:09 PM Md Haris Iqbal <haris.phnx@gmail.com> wrote:
>
> Currently rxe_invalidate_mr does invalidate for both local ops, and remote
> ones. This means that MR being invalidated is compared with rkey for both,
> which is incorrect. For local invalidate, comparison should happen with
> lkey, and for the remote one, it should happen with rkey.
>
> This commit splits the rxe_invalidate_mr into local and remote versions.
> Each of them does comparison the right way as described above (with lkey
> for local, and rkey for remote).
>
> Fixes: 3902b429ca14 ("RDMA/rxe: Implement invalidate MW operations")
> Cc: rpearsonhpe@gmail.com
> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com>
> ---
> v1 -> v2
> give a better name to key variable in function rxe_do_local_ops
>
> drivers/infiniband/sw/rxe/rxe_loc.h | 3 +-
> drivers/infiniband/sw/rxe/rxe_mr.c | 59 +++++++++++++++++++++-------
> drivers/infiniband/sw/rxe/rxe_req.c | 10 ++---
> drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
> 4 files changed, 53 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 0e022ae1b8a5..4da57abbbc8c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -77,7 +77,8 @@ struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
> enum rxe_mr_lookup_type type);
> int mr_check_range(struct rxe_mr *mr, u64 iova, size_t length);
> int advance_dma_data(struct rxe_dma_info *dma, unsigned int length);
> -int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey);
> +int rxe_invalidate_mr_local(struct rxe_qp *qp, u32 lkey);
> +int rxe_invalidate_mr_remote(struct rxe_qp *qp, u32 rkey);
> int rxe_reg_fast_mr(struct rxe_qp *qp, struct rxe_send_wqe *wqe);
> int rxe_mr_set_page(struct ib_mr *ibmr, u64 addr);
> int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata);
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index fc3942e04a1f..1c7179dd92eb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -576,41 +576,72 @@ struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
> return mr;
> }
>
> -int rxe_invalidate_mr(struct rxe_qp *qp, u32 rkey)
> +static int rxe_invalidate_mr(struct rxe_mr *mr)
> +{
> + if (atomic_read(&mr->num_mw) > 0) {
> + pr_warn("%s: Attempt to invalidate an MR while bound to MWs\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + if (unlikely(mr->type != IB_MR_TYPE_MEM_REG)) {
> + pr_warn("%s: mr->type (%d) is wrong type\n", __func__, mr->type);
> + return -EINVAL;
> + }
> +
> + mr->state = RXE_MR_STATE_FREE;
> + return 0;
> +}
> +
> +int rxe_invalidate_mr_local(struct rxe_qp *qp, u32 lkey)
> {
> struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> struct rxe_mr *mr;
> int ret;
>
> - mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
> + mr = rxe_pool_get_index(&rxe->mr_pool, lkey >> 8);
> if (!mr) {
> - pr_err("%s: No MR for rkey %#x\n", __func__, rkey);
> + pr_err("%s: No MR for lkey %#x\n", __func__, lkey);
> ret = -EINVAL;
> goto err;
> }
>
> - if (rkey != mr->rkey) {
> - pr_err("%s: rkey (%#x) doesn't match mr->rkey (%#x)\n",
> - __func__, rkey, mr->rkey);
> + if (lkey != mr->lkey) {
> + pr_err("%s: lkey (%#x) doesn't match mr->lkey (%#x)\n",
> + __func__, lkey, mr->lkey);
> ret = -EINVAL;
> goto err_drop_ref;
> }
>
> - if (atomic_read(&mr->num_mw) > 0) {
> - pr_warn("%s: Attempt to invalidate an MR while bound to MWs\n",
> - __func__);
> + ret = rxe_invalidate_mr(mr);
> +
> +err_drop_ref:
> + rxe_put(mr);
> +err:
> + return ret;
> +}
> +
> +int rxe_invalidate_mr_remote(struct rxe_qp *qp, u32 rkey)
> +{
> + struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> + struct rxe_mr *mr;
> + int ret;
> +
> + mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
> + if (!mr) {
> + pr_err("%s: No MR for rkey %#x\n", __func__, rkey);
> ret = -EINVAL;
> - goto err_drop_ref;
> + goto err;
> }
>
> - if (unlikely(mr->type != IB_MR_TYPE_MEM_REG)) {
> - pr_warn("%s: mr->type (%d) is wrong type\n", __func__, mr->type);
> + if (rkey != mr->rkey) {
> + pr_err("%s: rkey (%#x) doesn't match mr->rkey (%#x)\n",
> + __func__, rkey, mr->rkey);
> ret = -EINVAL;
> goto err_drop_ref;
> }
>
> - mr->state = RXE_MR_STATE_FREE;
> - ret = 0;
> + ret = rxe_invalidate_mr(mr);
>
> err_drop_ref:
> rxe_put(mr);
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 9d98237389cf..ef193a8a7158 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -541,16 +541,16 @@ static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
> static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
> {
> u8 opcode = wqe->wr.opcode;
> - u32 rkey;
> + u32 key;
> int ret;
>
> switch (opcode) {
> case IB_WR_LOCAL_INV:
> - rkey = wqe->wr.ex.invalidate_rkey;
> - if (rkey_is_mw(rkey))
> - ret = rxe_invalidate_mw(qp, rkey);
> + key = wqe->wr.ex.invalidate_rkey;
> + if (rkey_is_mw(key))
> + ret = rxe_invalidate_mw(qp, key);
> else
> - ret = rxe_invalidate_mr(qp, rkey);
> + ret = rxe_invalidate_mr_local(qp, key);
>
> if (unlikely(ret)) {
> wqe->status = IB_WC_LOC_QP_OP_ERR;
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index f4f6ee5d81fe..01411280cd73 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -818,7 +818,7 @@ static int invalidate_rkey(struct rxe_qp *qp, u32 rkey)
> if (rkey_is_mw(rkey))
> return rxe_invalidate_mw(qp, rkey);
> else
> - return rxe_invalidate_mr(qp, rkey);
> + return rxe_invalidate_mr_remote(qp, rkey);
> }
>
> /* Executes a new request. A retried request never reach that function (send
> --
> 2.25.1
>
ping.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions
2022-06-22 14:12 ` haris iqbal
@ 2022-06-24 20:02 ` Jason Gunthorpe
2022-06-27 0:33 ` Bob Pearson
0 siblings, 1 reply; 4+ messages in thread
From: Jason Gunthorpe @ 2022-06-24 20:02 UTC (permalink / raw)
To: haris iqbal
Cc: linux-rdma, Leon Romanovsky, zyjzyj2000, haris.iqbal, Jinpu Wang,
aleksei.marov, rpearsonhpe
On Wed, Jun 22, 2022 at 04:12:01PM +0200, haris iqbal wrote:
> On Thu, Jun 16, 2022 at 4:09 PM Md Haris Iqbal <haris.phnx@gmail.com> wrote:
> >
> > Currently rxe_invalidate_mr does invalidate for both local ops, and remote
> > ones. This means that MR being invalidated is compared with rkey for both,
> > which is incorrect. For local invalidate, comparison should happen with
> > lkey, and for the remote one, it should happen with rkey.
> >
> > This commit splits the rxe_invalidate_mr into local and remote versions.
> > Each of them does comparison the right way as described above (with lkey
> > for local, and rkey for remote).
> >
> > Fixes: 3902b429ca14 ("RDMA/rxe: Implement invalidate MW operations")
> > Cc: rpearsonhpe@gmail.com
> > Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com>
> ping.
The commit message makes sense to me - is anyone else working on rxe
going to test or review this patch?
There are now many patches for rxe that I would like the rxe community
to mutually review/test/ack.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions
2022-06-24 20:02 ` Jason Gunthorpe
@ 2022-06-27 0:33 ` Bob Pearson
0 siblings, 0 replies; 4+ messages in thread
From: Bob Pearson @ 2022-06-27 0:33 UTC (permalink / raw)
To: Jason Gunthorpe, haris iqbal
Cc: linux-rdma, Leon Romanovsky, zyjzyj2000, haris.iqbal, Jinpu Wang,
aleksei.marov
On 6/24/22 15:02, Jason Gunthorpe wrote:
> On Wed, Jun 22, 2022 at 04:12:01PM +0200, haris iqbal wrote:
>> On Thu, Jun 16, 2022 at 4:09 PM Md Haris Iqbal <haris.phnx@gmail.com> wrote:
>>>
>>> Currently rxe_invalidate_mr does invalidate for both local ops, and remote
>>> ones. This means that MR being invalidated is compared with rkey for both,
>>> which is incorrect. For local invalidate, comparison should happen with
>>> lkey, and for the remote one, it should happen with rkey.
>>>
>>> This commit splits the rxe_invalidate_mr into local and remote versions.
>>> Each of them does comparison the right way as described above (with lkey
>>> for local, and rkey for remote).
>>>
>>> Fixes: 3902b429ca14 ("RDMA/rxe: Implement invalidate MW operations")
>>> Cc: rpearsonhpe@gmail.com
>>> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com>
>
>> ping.
>
> The commit message makes sense to me - is anyone else working on rxe
> going to test or review this patch?
>
> There are now many patches for rxe that I would like the rxe community
> to mutually review/test/ack.
>
> Jason
I have reviewed this patch. Haris and I discussed it after v1 and we agreed to
this for v2.
Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-27 0:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 14:09 [PATCH v2] RDMA/rxe: Split rxe_invalidate_mr into local and remote versions Md Haris Iqbal
2022-06-22 14:12 ` haris iqbal
2022-06-24 20:02 ` Jason Gunthorpe
2022-06-27 0:33 ` Bob Pearson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.