From: Leon Romanovsky <leon@kernel.org>
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: Zhu Yanjun <zyjzyj2000@gmail.com>,
linux-rdma@vger.kernel.org, Bob Pearson <rpearson@hpe.com>
Subject: Re: [PATCH v3 17/17] rdma_rxe: minor cleanups
Date: Mon, 24 Aug 2020 12:02:37 +0300 [thread overview]
Message-ID: <20200824090237.GJ571722@unreal> (raw)
In-Reply-To: <abf9804c-8fc7-7f2d-f73d-0c9260f5cf60@gmail.com>
On Fri, Aug 21, 2020 at 11:05:43PM -0500, Bob Pearson wrote:
> On 8/21/20 9:14 PM, Zhu Yanjun wrote:
> > On 8/21/2020 6:46 AM, Bob Pearson wrote:
> >> Added vendor_part_id
> >> Fixed bug in rxe_resp.c at RKEY_VIOLATION: failed to ack bad rkeys
> >> Fixed failure to init mr in rxe_resp.c at check_rkey
> >> Fixed failure to allow invalidation of invalid MWs
> >>
> >> Signed-off-by: Bob Pearson <rpearson@hpe.com>
> >> ---
> >> drivers/infiniband/sw/rxe/rxe.c | 1 +
> >> drivers/infiniband/sw/rxe/rxe_mw.c | 20 ++++++++++++--------
> >> drivers/infiniband/sw/rxe/rxe_param.h | 1 +
> >> drivers/infiniband/sw/rxe/rxe_resp.c | 9 ++++++---
> >> 4 files changed, 20 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> >> index 10f441ac7203..11b043edd647 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe.c
> >> +++ b/drivers/infiniband/sw/rxe/rxe.c
> >> @@ -43,6 +43,7 @@ static void rxe_init_device_param(struct rxe_dev *rxe)
> >> rxe->max_inline_data = RXE_MAX_INLINE_DATA;
> >> rxe->attr.vendor_id = RXE_VENDOR_ID;
> >> + rxe->attr.vendor_part_id = RXE_VENDOR_PART_ID;
> >> rxe->attr.max_mr_size = RXE_MAX_MR_SIZE;
> >> rxe->attr.page_size_cap = RXE_PAGE_SIZE_CAP;
> >> rxe->attr.max_qp = RXE_MAX_QP;
> >> diff --git a/drivers/infiniband/sw/rxe/rxe_mw.c b/drivers/infiniband/sw/rxe/rxe_mw.c
> >> index 4178cf501a2b..a443deae35a3 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe_mw.c
> >> +++ b/drivers/infiniband/sw/rxe/rxe_mw.c
> >> @@ -28,7 +28,6 @@ static void rxe_set_mw_rkey(struct rxe_mw *mw)
> >> pr_err_once("unable to get random rkey for mw\n");
> >> }
> >> -/* this temporary code to test ibv_alloc_mw, ibv_dealloc_mw */
> >> struct ib_mw *rxe_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
> >> struct ib_udata *udata)
> >> {
> >> @@ -326,9 +325,12 @@ int rxe_bind_mw(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
> >> static int check_invalidate_mw(struct rxe_qp *qp, struct rxe_mw *mw)
> >> {
> >> - if (unlikely(mw->state != RXE_MEM_STATE_VALID)) {
> >> - pr_err_once("attempt to invalidate a MW that is not valid\n");
> >> - return -EINVAL;
> >> + /* run_test requires invalidate to work when !valid so don't check */
> >> + if (0) {
> >> + if (unlikely(mw->state != RXE_MEM_STATE_VALID)) {
> >> + pr_err_once("attempt to invalidate a MW that is not valid\n");
> >> + return -EINVAL;
> >> + }
> >> }
> >> /* o10-37.2.26 */
> >> @@ -344,9 +346,11 @@ static void do_invalidate_mw(struct rxe_mw *mw)
> >> {
> >> mw->qp = NULL;
> >> - rxe_drop_ref(mw->mr);
> >> - atomic_dec(&mw->mr->num_mw);
> >> - mw->mr = NULL;
> >> + if (mw->mr) {
> >> + rxe_drop_ref(mw->mr);
> >> + atomic_dec(&mw->mr->num_mw);
> >> + mw->mr = NULL;
> >> + }
> >> mw->access = 0;
> >> mw->addr = 0;
> >> @@ -378,7 +382,7 @@ int rxe_mw_check_access(struct rxe_qp *qp, struct rxe_mw *mw,
> >> struct rxe_pd *pd = to_rpd(mw->ibmw.pd);
> >> if (unlikely(mw->state != RXE_MEM_STATE_VALID)) {
> >> - pr_err_once("attempt to access a MW that is not in the valid state\n");
> >> + pr_err_once("attempt to access a MW that is not valid\n");
> >> return -EINVAL;
> >> }
> >> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> >> index e1d485bdd6af..beaa3f844819 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe_param.h
> >> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> >> @@ -107,6 +107,7 @@ enum rxe_device_param {
> >> /* IBTA v1.4 A3.3.1 VENDOR INFORMATION section */
> >> RXE_VENDOR_ID = 0XFFFFFF,
> >> + RXE_VENDOR_PART_ID = 1,
> >
> > RXE_VENDOR_PART_ID can be zero.
> >
> > Zhu Yanjun
> >
> >> };
> >> /* default/initial rxe port parameters */
> >> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> >> index d6e957a34910..bf7ef56aaf1c 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> >> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> >> @@ -392,8 +392,8 @@ static enum resp_states check_length(struct rxe_qp *qp,
> >> static enum resp_states check_rkey(struct rxe_qp *qp,
> >> struct rxe_pkt_info *pkt)
> >> {
> >> - struct rxe_mr *mr;
> >> - struct rxe_mw *mw;
> >> + struct rxe_mr *mr = NULL;
> >> + struct rxe_mw *mw = NULL;
> >> struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> >> u64 va;
> >> u32 rkey;
> >> @@ -1368,7 +1368,10 @@ int rxe_responder(void *arg)
> >> /* Class C */
> >> do_class_ac_error(qp, AETH_NAK_REM_ACC_ERR,
> >> IB_WC_REM_ACCESS_ERR);
> >> - state = RESPST_COMPLETE;
> >> + if (qp->resp.wqe)
> >> + state = RESPST_COMPLETE;
> >> + else
> >> + state = RESPST_ACKNOWLEDGE;
> >> } else {
> >> qp->resp.drop_msg = 1;
> >> if (qp->srq) {
> >
> >
> >
> I would agree but the pyverbs tests do not agree. If someone will fix the test I would be happy to leave it zero.
So please fix the pyverbs, together with SIW.
I don't like the idea of setting vendor_part_id randomly.
From IBTA, "A3.3.1 VENDOR INFORMATION":
A vendor that produces a generic controller (i.e., one that supports a standard
I/O protocol such as SRP), which does not have vendor specific device drivers,
may use the value of 0xFFFFFF in the VendorID field.
However, such a value prevents the vendor from ever providing vendor specific
drivers for the product
Thanks
next prev parent reply other threads:[~2020-08-24 9:03 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-20 22:46 [PATCH v3 00/17] Memory window support for rdma_rxe Bob Pearson
2020-08-20 22:46 ` [PATCH v3 01/17] rdma_rxe: Added SPDX headers to rxe source files Bob Pearson
2020-08-24 10:03 ` Leon Romanovsky
2020-08-20 22:46 ` [PATCH v3 02/17] rdma_rxe: Fixed style warnings Bob Pearson
2020-08-27 12:52 ` Jason Gunthorpe
2020-08-20 22:46 ` [PATCH v3 03/17] ib_user_verbs.h: Added ib_uverbs_wc_opcode Bob Pearson
2020-08-27 12:53 ` Jason Gunthorpe
2020-08-20 22:46 ` [PATCH v3 04/17] ib_verbs.h: Added missing IB_WR_BIND_MW opcode Bob Pearson
2020-08-27 12:54 ` Jason Gunthorpe
2020-08-20 22:46 ` [PATCH v3 05/17] rdma_rxe: Added bind_mw parameters to rxe_send_wr Bob Pearson
2020-08-27 12:56 ` Jason Gunthorpe
2020-08-20 22:46 ` [PATCH v3 06/17] rdma_rxe: Added stubs for alloc_mw and dealloc_mw verbs Bob Pearson
2020-08-27 12:56 ` Jason Gunthorpe
2020-08-20 22:46 ` [PATCH v3 07/17] rdma_rxe: Separated MR and MW objects Bob Pearson
2020-08-20 22:46 ` [PATCH v3 08/17] rdma_rxe: Added mw object Bob Pearson
2020-08-22 3:39 ` Zhu Yanjun
2020-08-20 22:46 ` [PATCH v3 09/17] rdma_rxe: Extended pools to support both keys and indices Bob Pearson
2020-08-20 22:46 ` [PATCH v3 10/17] rdma_rxe: Implemented functional alloc_mw and dealloc_mw APIs Bob Pearson
2020-08-20 22:46 ` [PATCH v3 11/17] rdma_rxe: Address an issue with hardened user copy Bob Pearson
2020-08-22 3:32 ` Zhu Yanjun
2020-08-22 4:16 ` Bob Pearson
2020-08-24 8:47 ` Leon Romanovsky
2020-08-24 8:52 ` Leon Romanovsky
2020-08-24 23:52 ` Bob Pearson
2020-08-25 5:04 ` Leon Romanovsky
2020-08-20 22:46 ` [PATCH v3 12/17] rdma_rxe: Added bind mw API stub Bob Pearson
2020-08-20 22:46 ` [PATCH v3 13/17] rdma_rxe: Give MR and MW objects indices and keys Bob Pearson
2020-08-20 22:46 ` [PATCH v3 14/17] rdma_rxe: Added stub for invalidate mw Bob Pearson
2020-08-20 22:46 ` [PATCH v3 15/17] rdma_rxe: Added functional bind and invalidate MW ops Bob Pearson
2020-08-20 22:46 ` [PATCH v3 16/17] rdma_rxe: Implemented read/write/atomic access to MW Bob Pearson
2020-08-20 22:46 ` [PATCH v3 17/17] rdma_rxe: minor cleanups Bob Pearson
[not found] ` <a153a775-9b53-3ccc-4c2a-ec76f863d1a1@gmail.com>
2020-08-22 4:05 ` Bob Pearson
2020-08-24 9:02 ` Leon Romanovsky [this message]
2020-08-27 13:00 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200824090237.GJ571722@unreal \
--to=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=rpearson@hpe.com \
--cc=rpearsonhpe@gmail.com \
--cc=zyjzyj2000@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.