[PATCH] smb: smbdirect: fix MR registration for coalesced SG lists

[PATCH] smb: smbdirect: fix MR registration for coalesced SG lists

[Yi Kuo]

ib_dma_map_sg() modifies the provided scatterlist and returns the
number of mapped entries, which can be fewer than the requested
mr->sgt.nents if the DMA controller coalesces contiguous memory
segments. Passing the original, uncoalesced count to ib_map_mr_sg()
causes memory registration failures if coalescing actually occurs.

Capture the actual mapped count returned by ib_dma_map_sg() and pass it
to ib_map_mr_sg() to ensure correct MR registration.

Also update the ib_dma_map_sg() error logging to drop the error
pointer formatting, since the return value is an integer count
rather than an error code.

Signed-off-by: Yi Kuo <yi@yikuo.dev>
---
 fs/smb/smbdirect/mr.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/smb/smbdirect/mr.c b/fs/smb/smbdirect/mr.c
index 5228e699cd5d..058dc24bf544 100644
--- a/fs/smb/smbdirect/mr.c
+++ b/fs/smb/smbdirect/mr.c
@@ -269,7 +269,7 @@ smbdirect_connection_register_mr_io(struct smbdirect_socket *sc,
 {
 	const struct smbdirect_socket_parameters *sp = &sc->parameters;
 	struct smbdirect_mr_io *mr;
-	int ret, num_pages;
+	int ret, num_pages, num_mapped;
 	struct ib_reg_wr *reg_wr;
 
 	num_pages = iov_iter_npages(iter, sp->max_frmr_depth + 1);
@@ -300,19 +300,19 @@ smbdirect_connection_register_mr_io(struct smbdirect_socket *sc,
 		num_pages, iov_iter_count(iter), sp->max_frmr_depth);
 	smbdirect_iter_to_sgt(iter, &mr->sgt, sp->max_frmr_depth);
 
-	ret = ib_dma_map_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
-	if (!ret) {
+	num_mapped = ib_dma_map_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
+	if (!num_mapped) {
 		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
-			"ib_dma_map_sg num_pages=%u dir=%x ret=%d (%1pe)\n",
-			num_pages, mr->dir, ret, SMBDIRECT_DEBUG_ERR_PTR(ret));
+			"ib_dma_map_sg num_pages=%u dir=%x num_mapped=%d\n",
+			num_pages, mr->dir, num_mapped);
 		goto dma_map_error;
 	}
 
-	ret = ib_map_mr_sg(mr->mr, mr->sgt.sgl, mr->sgt.nents, NULL, PAGE_SIZE);
-	if (ret != mr->sgt.nents) {
+	ret = ib_map_mr_sg(mr->mr, mr->sgt.sgl, num_mapped, NULL, PAGE_SIZE);
+	if (ret != num_mapped) {
 		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
-			"ib_map_mr_sg failed ret = %d nents = %u\n",
-			ret, mr->sgt.nents);
+			"ib_map_mr_sg failed ret = %d num_mapped = %u\n",
+			ret, num_mapped);
 		goto map_mr_error;
 	}
 
-- 
2.54.0

Re: [PATCH] smb: smbdirect: fix MR registration for coalesced SG lists

[Stefan Metzmacher]

Hi Yi,

> ib_dma_map_sg() modifies the provided scatterlist and returns the
> number of mapped entries, which can be fewer than the requested
> mr->sgt.nents if the DMA controller coalesces contiguous memory
> segments. Passing the original, uncoalesced count to ib_map_mr_sg()
> causes memory registration failures if coalescing actually occurs.
> 
> Capture the actual mapped count returned by ib_dma_map_sg() and pass it
> to ib_map_mr_sg() to ensure correct MR registration.
> 
> Also update the ib_dma_map_sg() error logging to drop the error
> pointer formatting, since the return value is an integer count
> rather than an error code.
> 
> Signed-off-by: Yi Kuo <yi@yikuo.dev>
> ---
>   fs/smb/smbdirect/mr.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/smb/smbdirect/mr.c b/fs/smb/smbdirect/mr.c
> index 5228e699cd5d..058dc24bf544 100644
> --- a/fs/smb/smbdirect/mr.c
> +++ b/fs/smb/smbdirect/mr.c
> @@ -269,7 +269,7 @@ smbdirect_connection_register_mr_io(struct smbdirect_socket *sc,
>   {
>   	const struct smbdirect_socket_parameters *sp = &sc->parameters;
>   	struct smbdirect_mr_io *mr;
> -	int ret, num_pages;
> +	int ret, num_pages, num_mapped;
>   	struct ib_reg_wr *reg_wr;
>   
>   	num_pages = iov_iter_npages(iter, sp->max_frmr_depth + 1);
> @@ -300,19 +300,19 @@ smbdirect_connection_register_mr_io(struct smbdirect_socket *sc,
>   		num_pages, iov_iter_count(iter), sp->max_frmr_depth);
>   	smbdirect_iter_to_sgt(iter, &mr->sgt, sp->max_frmr_depth);
>   
> -	ret = ib_dma_map_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
> -	if (!ret) {
> +	num_mapped = ib_dma_map_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
> +	if (!num_mapped) {
>   		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
> -			"ib_dma_map_sg num_pages=%u dir=%x ret=%d (%1pe)\n",
> -			num_pages, mr->dir, ret, SMBDIRECT_DEBUG_ERR_PTR(ret));
> +			"ib_dma_map_sg num_pages=%u dir=%x num_mapped=%d\n",
> +			num_pages, mr->dir, num_mapped);

While here we should have ret = -EIO;

>   		goto dma_map_error;
>   	}

Thanks! I didn't notice this and tried to fix it with some other logic:
https://git.samba.org/?p=metze/linux/wip.git;a=commitdiff;h=11fe681cb8a36a8e48366cc5dd6dff37c1de350e

> -	ret = ib_map_mr_sg(mr->mr, mr->sgt.sgl, mr->sgt.nents, NULL, PAGE_SIZE);
> -	if (ret != mr->sgt.nents) {
> +	ret = ib_map_mr_sg(mr->mr, mr->sgt.sgl, num_mapped, NULL, PAGE_SIZE);
> +	if (ret != num_mapped) {
>   		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
> -			"ib_map_mr_sg failed ret = %d nents = %u\n",
> -			ret, mr->sgt.nents);
> +			"ib_map_mr_sg failed ret = %d num_mapped = %u\n",
> +			ret, num_mapped);

I guess we want something like this:

		if (ret >= 0)
			ret = -EIO;

>   		goto map_mr_error;
>   	}

This should also have

Fixes: de5ef8ec3c46 ("smb: smbdirect: introduce smbdirect_mr.c with client mr code")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221408

Thanks!
metze

Re: [PATCH] smb: smbdirect: fix MR registration for coalesced SG lists

[kernel test robot]

Hi Yi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v7.1-rc1]
[also build test WARNING on linus/master next-20260429]
[cannot apply to brauner-vfs/vfs.all]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yi-Kuo/smb-smbdirect-fix-MR-registration-for-coalesced-SG-lists/20260429-221606
base:   v7.1-rc1
patch link:    https://lore.kernel.org/r/4bf423bb.AU4AAJbgzlAAAAAAAAAABAMtJJMAAYKKIjQAAAAAAC-ZZgBp8Ww0%40mailjet.com
patch subject: [PATCH] smb: smbdirect: fix MR registration for coalesced SG lists
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260430/202604301612.XQrb9dxs-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260430/202604301612.XQrb9dxs-lkp@intel.com/reproduce)

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/202604301612.XQrb9dxs-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/smb/smbdirect/mr.c:304:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     304 |         if (!num_mapped) {
         |             ^~~~~~~~~~~
   fs/smb/smbdirect/mr.c:361:40: note: uninitialized use occurs here
     361 |         smbdirect_socket_schedule_cleanup(sc, ret);
         |                                               ^~~
   fs/smb/smbdirect/internal.h:63:23: note: expanded from macro 'smbdirect_socket_schedule_cleanup'
      63 |                 __func__, __LINE__, __error, NULL)
         |                                     ^~~~~~~
   fs/smb/smbdirect/mr.c:304:2: note: remove the 'if' if its condition is always false
     304 |         if (!num_mapped) {
         |         ^~~~~~~~~~~~~~~~~~
     305 |                 smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     306 |                         "ib_dma_map_sg num_pages=%u dir=%x num_mapped=%d\n",
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     307 |                         num_pages, mr->dir, num_mapped);
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     308 |                 goto dma_map_error;
         |                 ~~~~~~~~~~~~~~~~~~~
     309 |         }
         |         ~
   fs/smb/smbdirect/mr.c:272:9: note: initialize the variable 'ret' to silence this warning
     272 |         int ret, num_pages, num_mapped;
         |                ^
         |                 = 0
   1 warning generated.


vim +304 fs/smb/smbdirect/mr.c

   256	
   257	/*
   258	 * Register memory for RDMA read/write
   259	 * iter: the buffer to register memory with
   260	 * writing: true if this is a RDMA write (SMB read), false for RDMA read
   261	 * need_invalidate: true if this MR needs to be locally invalidated after I/O
   262	 * return value: the MR registered, NULL if failed.
   263	 */
   264	struct smbdirect_mr_io *
   265	smbdirect_connection_register_mr_io(struct smbdirect_socket *sc,
   266					    struct iov_iter *iter,
   267					    bool writing,
   268					    bool need_invalidate)
   269	{
   270		const struct smbdirect_socket_parameters *sp = &sc->parameters;
   271		struct smbdirect_mr_io *mr;
   272		int ret, num_pages, num_mapped;
   273		struct ib_reg_wr *reg_wr;
   274	
   275		num_pages = iov_iter_npages(iter, sp->max_frmr_depth + 1);
   276		if (num_pages > sp->max_frmr_depth) {
   277			smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
   278				"num_pages=%d max_frmr_depth=%d\n",
   279				num_pages, sp->max_frmr_depth);
   280			WARN_ON_ONCE(1);
   281			return NULL;
   282		}
   283	
   284		mr = smbdirect_connection_get_mr_io(sc);
   285		if (!mr) {
   286			smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
   287				"smbdirect_connection_get_mr_io returning NULL\n");
   288			return NULL;
   289		}
   290	
   291		mutex_lock(&mr->mutex);
   292	
   293		mr->dir = writing ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
   294		mr->need_invalidate = need_invalidate;
   295		mr->sgt.nents = 0;
   296		mr->sgt.orig_nents = 0;
   297	
   298		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_INFO,
   299			"num_pages=%u count=%zu depth=%u\n",
   300			num_pages, iov_iter_count(iter), sp->max_frmr_depth);
   301		smbdirect_iter_to_sgt(iter, &mr->sgt, sp->max_frmr_depth);
   302	
   303		num_mapped = ib_dma_map_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
 > 304		if (!num_mapped) {
   305			smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
   306				"ib_dma_map_sg num_pages=%u dir=%x num_mapped=%d\n",
   307				num_pages, mr->dir, num_mapped);
   308			goto dma_map_error;
   309		}
   310	
   311		ret = ib_map_mr_sg(mr->mr, mr->sgt.sgl, num_mapped, NULL, PAGE_SIZE);
   312		if (ret != num_mapped) {
   313			smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
   314				"ib_map_mr_sg failed ret = %d num_mapped = %u\n",
   315				ret, num_mapped);
   316			goto map_mr_error;
   317		}
   318	
   319		ib_update_fast_reg_key(mr->mr, ib_inc_rkey(mr->mr->rkey));
   320		reg_wr = &mr->wr;
   321		reg_wr->wr.opcode = IB_WR_REG_MR;
   322		mr->cqe.done = smbdirect_connection_mr_io_register_done;
   323		reg_wr->wr.wr_cqe = &mr->cqe;
   324		reg_wr->wr.num_sge = 0;
   325		reg_wr->wr.send_flags = IB_SEND_SIGNALED;
   326		reg_wr->mr = mr->mr;
   327		reg_wr->key = mr->mr->rkey;
   328		reg_wr->access = writing ?
   329				IB_ACCESS_REMOTE_WRITE | IB_ACCESS_LOCAL_WRITE :
   330				IB_ACCESS_REMOTE_READ;
   331	
   332		/*
   333		 * There is no need for waiting for complemtion on ib_post_send
   334		 * on IB_WR_REG_MR. Hardware enforces a barrier and order of execution
   335		 * on the next ib_post_send when we actually send I/O to remote peer
   336		 */
   337		ret = ib_post_send(sc->ib.qp, &reg_wr->wr, NULL);
   338		if (!ret) {
   339			/*
   340			 * smbdirect_connection_get_mr_io() gave us a reference
   341			 * via kref_get(&mr->kref), we keep that and let
   342			 * the caller use smbdirect_connection_deregister_mr_io()
   343			 * to remove it again.
   344			 */
   345			mutex_unlock(&mr->mutex);
   346			return mr;
   347		}
   348	
   349		smbdirect_log_rdma_mr(sc, SMBDIRECT_LOG_ERR,
   350			"ib_post_send failed ret=%d (%1pe) reg_wr->key=0x%x\n",
   351			ret, SMBDIRECT_DEBUG_ERR_PTR(ret), reg_wr->key);
   352	
   353	map_mr_error:
   354		ib_dma_unmap_sg(sc->ib.dev, mr->sgt.sgl, mr->sgt.nents, mr->dir);
   355	
   356	dma_map_error:
   357		mr->sgt.nents = 0;
   358		mr->state = SMBDIRECT_MR_ERROR;
   359		atomic_dec(&sc->mr_io.used.count);
   360	
   361		smbdirect_socket_schedule_cleanup(sc, ret);
   362	
   363		/*
   364		 * smbdirect_connection_get_mr_io() gave us a reference
   365		 * via kref_get(&mr->kref), we need to remove it again
   366		 * on error.
   367		 *
   368		 * No kref_put_mutex() as it's already locked.
   369		 *
   370		 * If smbdirect_mr_io_free_locked() is called
   371		 * and the mutex is unlocked and mr is gone,
   372		 * in that case kref_put() returned 1.
   373		 *
   374		 * If kref_put() returned 0 we know that
   375		 * smbdirect_mr_io_free_locked() didn't
   376		 * run. Not by us nor by anyone else, as we
   377		 * still hold the mutex, so we need to unlock.
   378		 */
   379		if (!kref_put(&mr->kref, smbdirect_mr_io_free_locked))
   380			mutex_unlock(&mr->mutex);
   381		return NULL;
   382	}
   383	__SMBDIRECT_EXPORT_SYMBOL__(smbdirect_connection_register_mr_io);
   384	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki