diff options
| author | Gerd Rausch <gerd.rausch@oracle.com> | 2019-06-29 02:31:19 +0200 |
|---|---|---|
| committer | Santosh Shilimkar <santosh.shilimkar@oracle.com> | 2019-07-10 06:45:41 +0200 |
| commit | a55207884708bf1d5b8c87a3c504502de77a5416 (patch) | |
| tree | 65b06062d9c6f97fc55c68d15abfc63bc18106a8 /net/rds/ib_frmr.c | |
| parent | rds: fix reordering with composite message notification (diff) | |
| download | linux-a55207884708bf1d5b8c87a3c504502de77a5416.tar.xz linux-a55207884708bf1d5b8c87a3c504502de77a5416.zip | |
Revert "RDS: IB: split the mr registration and invalidation path"
This reverts commit 56012459310a1dbcc55c2dbf5500a9f7571402cb.
RDS kept spinning inside function "rds_ib_post_reg_frmr", waiting for
"i_fastreg_wrs" to become incremented:
while (atomic_dec_return(&ibmr->ic->i_fastreg_wrs) <= 0) {
atomic_inc(&ibmr->ic->i_fastreg_wrs);
cpu_relax();
}
Looking at the original commit:
commit 56012459310a ("RDS: IB: split the mr registration and
invalidation path")
In there, the "rds_ib_mr_cqe_handler" was changed in the following
way:
void rds_ib_mr_cqe_handler(struct
rds_ib_connection *ic,
struct ib_wc *wc)
if (frmr->fr_inv) {
frmr->fr_state = FRMR_IS_FREE;
frmr->fr_inv = false;
atomic_inc(&ic->i_fastreg_wrs);
} else {
atomic_inc(&ic->i_fastunreg_wrs);
}
It looks like it's got it exactly backwards:
Function "rds_ib_post_reg_frmr" keeps track of the outstanding
requests via "i_fastreg_wrs".
Function "rds_ib_post_inv" keeps track of the outstanding requests
via "i_fastunreg_wrs" (post original commit). It also sets:
frmr->fr_inv = true;
However the completion handler "rds_ib_mr_cqe_handler" adjusts
"i_fastreg_wrs" when "fr_inv" had been true, and adjusts
"i_fastunreg_wrs" otherwise.
The original commit was done in the name of performance:
to remove the performance bottleneck
No performance benefit could be observed with a fixed-up version
of the original commit measured between two Oracle X7 servers,
both equipped with Mellanox Connect-X5 HCAs.
The prudent course of action is to revert this commit.
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Diffstat (limited to 'net/rds/ib_frmr.c')
| -rw-r--r-- | net/rds/ib_frmr.c | 11 |
1 files changed, 5 insertions, 6 deletions
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c index 688dcd68d4ea..32ae26ed58a0 100644 --- a/net/rds/ib_frmr.c +++ b/net/rds/ib_frmr.c @@ -239,8 +239,8 @@ static int rds_ib_post_inv(struct rds_ib_mr *ibmr) if (frmr->fr_state != FRMR_IS_INUSE) goto out; - while (atomic_dec_return(&ibmr->ic->i_fastunreg_wrs) <= 0) { - atomic_inc(&ibmr->ic->i_fastunreg_wrs); + while (atomic_dec_return(&ibmr->ic->i_fastreg_wrs) <= 0) { + atomic_inc(&ibmr->ic->i_fastreg_wrs); cpu_relax(); } @@ -257,7 +257,7 @@ static int rds_ib_post_inv(struct rds_ib_mr *ibmr) if (unlikely(ret)) { frmr->fr_state = FRMR_IS_STALE; frmr->fr_inv = false; - atomic_inc(&ibmr->ic->i_fastunreg_wrs); + atomic_inc(&ibmr->ic->i_fastreg_wrs); pr_err("RDS/IB: %s returned error(%d)\n", __func__, ret); goto out; } @@ -285,10 +285,9 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc) if (frmr->fr_inv) { frmr->fr_state = FRMR_IS_FREE; frmr->fr_inv = false; - atomic_inc(&ic->i_fastreg_wrs); - } else { - atomic_inc(&ic->i_fastunreg_wrs); } + + atomic_inc(&ic->i_fastreg_wrs); } void rds_ib_unreg_frmr(struct list_head *list, unsigned int *nfreed, |