We need to increment i_fastreg_wrs before we bail out from rds_ib_post_reg_frmr().
We have a fixed budget of how many FRWR operations that can be outstanding using the dedicated QP used for memory registrations and de-registrations. This budget is enforced by the atomic_t i_fastreg_wrs. If we bail out early in rds_ib_post_reg_frmr(), we will "leak" the possibility of posting an FRWR operation, and if that accumulates, no FRWR operation can be carried out.
Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode") Fixes: 3a2886cca703 ("net/rds: Keep track of and wait for FRWR segments in use upon shutdown") Cc: stable@vger.kernel.org Signed-off-by: Håkon Bugge haakon.bugge@oracle.com
---
v2 -> v3: * Amended commit message * Removed indentation of this section * Fixing error path from ib_post_send()
v1 -> v2: Added Cc: stable@vger.kernel.org --- net/rds/ib_frmr.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c index 28c1b00221780..395a99b5a65ca 100644 --- a/net/rds/ib_frmr.c +++ b/net/rds/ib_frmr.c @@ -133,12 +133,15 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
ret = ib_map_mr_sg_zbva(frmr->mr, ibmr->sg, ibmr->sg_dma_len, &off, PAGE_SIZE); - if (unlikely(ret != ibmr->sg_dma_len)) - return ret < 0 ? ret : -EINVAL; + if (unlikely(ret != ibmr->sg_dma_len)) { + ret = ret < 0 ? ret : -EINVAL; + goto out_inc; + }
- if (cmpxchg(&frmr->fr_state, - FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE) - return -EBUSY; + if (cmpxchg(&frmr->fr_state, FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE) { + ret = -EBUSY; + goto out_inc; + }
atomic_inc(&ibmr->ic->i_fastreg_inuse_count);
@@ -166,11 +169,10 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) /* Failure here can be because of -ENOMEM as well */ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
- atomic_inc(&ibmr->ic->i_fastreg_wrs); if (printk_ratelimit()) pr_warn("RDS/IB: %s returned error(%d)\n", __func__, ret); - goto out; + goto out_inc; }
/* Wait for the registration to complete in order to prevent an invalid @@ -178,9 +180,11 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) * being accessed while registration is still pending. */ wait_event(frmr->fr_reg_done, !frmr->fr_reg); - out: + return ret;
+out_inc: + atomic_inc(&ibmr->ic->i_fastreg_wrs); return ret; }
On Wed, 2025-09-10 at 13:04 +0200, Håkon Bugge wrote:
We need to increment i_fastreg_wrs before we bail out from rds_ib_post_reg_frmr().
We have a fixed budget of how many FRWR operations that can be outstanding using the dedicated QP used for memory registrations and de-registrations. This budget is enforced by the atomic_t i_fastreg_wrs. If we bail out early in rds_ib_post_reg_frmr(), we will "leak" the possibility of posting an FRWR operation, and if that accumulates, no FRWR operation can be carried out.
Hi Håkon,
This sounds much clearer, thank you!
Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode") Fixes: 3a2886cca703 ("net/rds: Keep track of and wait for FRWR segments in use upon shutdown") Cc: stable@vger.kernel.org Signed-off-by: Håkon Bugge haakon.bugge@oracle.com
v2 -> v3:
- Amended commit message
- Removed indentation of this section
- Fixing error path from ib_post_send()
v1 -> v2: Added Cc: stable@vger.kernel.org
net/rds/ib_frmr.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c index 28c1b00221780..395a99b5a65ca 100644 --- a/net/rds/ib_frmr.c +++ b/net/rds/ib_frmr.c @@ -133,12 +133,15 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) ret = ib_map_mr_sg_zbva(frmr->mr, ibmr->sg, ibmr->sg_dma_len, &off, PAGE_SIZE);
- if (unlikely(ret != ibmr->sg_dma_len))
return ret < 0 ? ret : -EINVAL;
- if (unlikely(ret != ibmr->sg_dma_len)) {
ret = ret < 0 ? ret : -EINVAL;goto out_inc;- }
- if (cmpxchg(&frmr->fr_state,
FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE)return -EBUSY;
- if (cmpxchg(&frmr->fr_state, FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE) {
ret = -EBUSY;goto out_inc;- }
atomic_inc(&ibmr->ic->i_fastreg_inuse_count); @@ -166,11 +169,10 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) /* Failure here can be because of -ENOMEM as well */ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
atomic_inc(&ibmr->ic->i_fastreg_wrs);goto out;
Just one nit: This was the only place the out label was used isnt it? If so, lets go ahead and clear it out.
goto out_inc;/* Wait for the registration to complete in order to prevent an invalid @@ -178,9 +180,11 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) * being accessed while registration is still pending. */ wait_event(frmr->fr_reg_done, !frmr->fr_reg);
out:
With that fixed, you can go ahead and add my rvb: Reviewed-by: Allison Henderson allison.henderson@oracle.com
Thank you!
- return ret;
+out_inc:
- atomic_inc(&ibmr->ic->i_fastreg_wrs); return ret;
}
Hi Allison,
On 10 Sep 2025, at 20:47, Allison Henderson allison.henderson@oracle.com wrote:
On Wed, 2025-09-10 at 13:04 +0200, Håkon Bugge wrote:
We need to increment i_fastreg_wrs before we bail out from rds_ib_post_reg_frmr().
We have a fixed budget of how many FRWR operations that can be outstanding using the dedicated QP used for memory registrations and de-registrations. This budget is enforced by the atomic_t i_fastreg_wrs. If we bail out early in rds_ib_post_reg_frmr(), we will "leak" the possibility of posting an FRWR operation, and if that accumulates, no FRWR operation can be carried out.
Hi Håkon,
This sounds much clearer, thank you!
Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode") Fixes: 3a2886cca703 ("net/rds: Keep track of and wait for FRWR segments in use upon shutdown") Cc: stable@vger.kernel.org Signed-off-by: Håkon Bugge haakon.bugge@oracle.com
v2 -> v3:
- Amended commit message
- Removed indentation of this section
- Fixing error path from ib_post_send()
v1 -> v2: Added Cc: stable@vger.kernel.org
net/rds/ib_frmr.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c index 28c1b00221780..395a99b5a65ca 100644 --- a/net/rds/ib_frmr.c +++ b/net/rds/ib_frmr.c @@ -133,12 +133,15 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
ret = ib_map_mr_sg_zbva(frmr->mr, ibmr->sg, ibmr->sg_dma_len, &off, PAGE_SIZE);
- if (unlikely(ret != ibmr->sg_dma_len))
- return ret < 0 ? ret : -EINVAL;
- if (unlikely(ret != ibmr->sg_dma_len)) {
- ret = ret < 0 ? ret : -EINVAL;
- goto out_inc;
- }
- if (cmpxchg(&frmr->fr_state,
FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE)- return -EBUSY;
- if (cmpxchg(&frmr->fr_state, FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE) {
- ret = -EBUSY;
- goto out_inc;
- }
atomic_inc(&ibmr->ic->i_fastreg_inuse_count);
@@ -166,11 +169,10 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) /* Failure here can be because of -ENOMEM as well */ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
- atomic_inc(&ibmr->ic->i_fastreg_wrs);
if (printk_ratelimit()) pr_warn("RDS/IB: %s returned error(%d)\n", __func__, ret);
- goto out;
Just one nit: This was the only place the out label was used isnt it? If so, lets go ahead and clear it out.
That is indeed true!
- goto out_inc;
}
/* Wait for the registration to complete in order to prevent an invalid @@ -178,9 +180,11 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
- being accessed while registration is still pending.
*/ wait_event(frmr->fr_reg_done, !frmr->fr_reg);
out:
With that fixed, you can go ahead and add my rvb: Reviewed-by: Allison Henderson allison.henderson@oracle.com
Will do, thanks for the r-b!
Håkon
Thank you!
- return ret;
+out_inc:
- atomic_inc(&ibmr->ic->i_fastreg_wrs);
return ret; }
Hi Håkon,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/H-kon-Bugge/rds-ib-Increment-... base: net/main patch link: https://lore.kernel.org/r/20250910110501.350238-1-haakon.bugge%40oracle.com patch subject: [PATCH net v3] rds: ib: Increment i_fastreg_wrs before bailing out config: x86_64-buildonly-randconfig-001-20250911 (https://download.01.org/0day-ci/archive/20250911/202509110810.t8tVEIDs-lkp@i...) compiler: gcc-14 (Debian 14.2.0-19) 14.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250911/202509110810.t8tVEIDs-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202509110810.t8tVEIDs-lkp@intel.com/
All warnings (new ones prefixed by >>):
net/rds/ib_frmr.c: In function 'rds_ib_post_reg_frmr':
net/rds/ib_frmr.c:183:1: warning: label 'out' defined but not used [-Wunused-label]
183 | out: | ^~~
vim +/out +183 net/rds/ib_frmr.c
1659185fb4d002 Avinash Repaka 2016-03-01 122 1659185fb4d002 Avinash Repaka 2016-03-01 123 static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr) 1659185fb4d002 Avinash Repaka 2016-03-01 124 { 1659185fb4d002 Avinash Repaka 2016-03-01 125 struct rds_ib_frmr *frmr = &ibmr->u.frmr; 1659185fb4d002 Avinash Repaka 2016-03-01 126 struct ib_reg_wr reg_wr; 3e56c2f856d7ab Santosh Shilimkar 2016-12-04 127 int ret, off = 0; 1659185fb4d002 Avinash Repaka 2016-03-01 128 1659185fb4d002 Avinash Repaka 2016-03-01 129 while (atomic_dec_return(&ibmr->ic->i_fastreg_wrs) <= 0) { 1659185fb4d002 Avinash Repaka 2016-03-01 130 atomic_inc(&ibmr->ic->i_fastreg_wrs); 1659185fb4d002 Avinash Repaka 2016-03-01 131 cpu_relax(); 1659185fb4d002 Avinash Repaka 2016-03-01 132 } 1659185fb4d002 Avinash Repaka 2016-03-01 133 fb4b1373dcab08 Gerd Rausch 2021-08-17 134 ret = ib_map_mr_sg_zbva(frmr->mr, ibmr->sg, ibmr->sg_dma_len, 3e56c2f856d7ab Santosh Shilimkar 2016-12-04 135 &off, PAGE_SIZE); cc67d6c0924d12 Håkon Bugge 2025-09-10 136 if (unlikely(ret != ibmr->sg_dma_len)) { cc67d6c0924d12 Håkon Bugge 2025-09-10 137 ret = ret < 0 ? ret : -EINVAL; cc67d6c0924d12 Håkon Bugge 2025-09-10 138 goto out_inc; cc67d6c0924d12 Håkon Bugge 2025-09-10 139 } 1659185fb4d002 Avinash Repaka 2016-03-01 140 cc67d6c0924d12 Håkon Bugge 2025-09-10 141 if (cmpxchg(&frmr->fr_state, FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE) { cc67d6c0924d12 Håkon Bugge 2025-09-10 142 ret = -EBUSY; cc67d6c0924d12 Håkon Bugge 2025-09-10 143 goto out_inc; cc67d6c0924d12 Håkon Bugge 2025-09-10 144 } 3a2886cca703fd Gerd Rausch 2019-07-16 145 3a2886cca703fd Gerd Rausch 2019-07-16 146 atomic_inc(&ibmr->ic->i_fastreg_inuse_count); 3a2886cca703fd Gerd Rausch 2019-07-16 147 1659185fb4d002 Avinash Repaka 2016-03-01 148 /* Perform a WR for the fast_reg_mr. Each individual page 1659185fb4d002 Avinash Repaka 2016-03-01 149 * in the sg list is added to the fast reg page list and placed 1659185fb4d002 Avinash Repaka 2016-03-01 150 * inside the fast_reg_mr WR. The key used is a rolling 8bit 1659185fb4d002 Avinash Repaka 2016-03-01 151 * counter, which should guarantee uniqueness. 1659185fb4d002 Avinash Repaka 2016-03-01 152 */ 1659185fb4d002 Avinash Repaka 2016-03-01 153 ib_update_fast_reg_key(frmr->mr, ibmr->remap_count++); 5f33141d2fc05a Gerd Rausch 2019-07-16 154 frmr->fr_reg = true; 1659185fb4d002 Avinash Repaka 2016-03-01 155 1659185fb4d002 Avinash Repaka 2016-03-01 156 memset(®_wr, 0, sizeof(reg_wr)); 1659185fb4d002 Avinash Repaka 2016-03-01 157 reg_wr.wr.wr_id = (unsigned long)(void *)ibmr; 1659185fb4d002 Avinash Repaka 2016-03-01 158 reg_wr.wr.opcode = IB_WR_REG_MR; 1659185fb4d002 Avinash Repaka 2016-03-01 159 reg_wr.wr.num_sge = 0; 1659185fb4d002 Avinash Repaka 2016-03-01 160 reg_wr.mr = frmr->mr; 1659185fb4d002 Avinash Repaka 2016-03-01 161 reg_wr.key = frmr->mr->rkey; 1659185fb4d002 Avinash Repaka 2016-03-01 162 reg_wr.access = IB_ACCESS_LOCAL_WRITE | 1659185fb4d002 Avinash Repaka 2016-03-01 163 IB_ACCESS_REMOTE_READ | 1659185fb4d002 Avinash Repaka 2016-03-01 164 IB_ACCESS_REMOTE_WRITE; 1659185fb4d002 Avinash Repaka 2016-03-01 165 reg_wr.wr.send_flags = IB_SEND_SIGNALED; 1659185fb4d002 Avinash Repaka 2016-03-01 166 f112d53b435692 Bart Van Assche 2018-07-18 167 ret = ib_post_send(ibmr->ic->i_cm_id->qp, ®_wr.wr, NULL); 1659185fb4d002 Avinash Repaka 2016-03-01 168 if (unlikely(ret)) { 1659185fb4d002 Avinash Repaka 2016-03-01 169 /* Failure here can be because of -ENOMEM as well */ 3a2886cca703fd Gerd Rausch 2019-07-16 170 rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE); 3a2886cca703fd Gerd Rausch 2019-07-16 171 1659185fb4d002 Avinash Repaka 2016-03-01 172 if (printk_ratelimit()) 1659185fb4d002 Avinash Repaka 2016-03-01 173 pr_warn("RDS/IB: %s returned error(%d)\n", 1659185fb4d002 Avinash Repaka 2016-03-01 174 __func__, ret); cc67d6c0924d12 Håkon Bugge 2025-09-10 175 goto out_inc; 1659185fb4d002 Avinash Repaka 2016-03-01 176 } 5f33141d2fc05a Gerd Rausch 2019-07-16 177 5f33141d2fc05a Gerd Rausch 2019-07-16 178 /* Wait for the registration to complete in order to prevent an invalid 5f33141d2fc05a Gerd Rausch 2019-07-16 179 * access error resulting from a race between the memory region already 5f33141d2fc05a Gerd Rausch 2019-07-16 180 * being accessed while registration is still pending. 5f33141d2fc05a Gerd Rausch 2019-07-16 181 */ 5f33141d2fc05a Gerd Rausch 2019-07-16 182 wait_event(frmr->fr_reg_done, !frmr->fr_reg); 5f33141d2fc05a Gerd Rausch 2019-07-16 @183 out: cc67d6c0924d12 Håkon Bugge 2025-09-10 184 return ret; 5f33141d2fc05a Gerd Rausch 2019-07-16 185 cc67d6c0924d12 Håkon Bugge 2025-09-10 186 out_inc: cc67d6c0924d12 Håkon Bugge 2025-09-10 187 atomic_inc(&ibmr->ic->i_fastreg_wrs); 1659185fb4d002 Avinash Repaka 2016-03-01 188 return ret; 1659185fb4d002 Avinash Repaka 2016-03-01 189 } 1659185fb4d002 Avinash Repaka 2016-03-01 190
linux-stable-mirror@lists.linaro.org
-
Allison Henderson -
Haakon Bugge -
Håkon Bugge
-
kernel test robot