Re: [PATCH v2] IB: rework memlock limit handling code

[Leon Romanovsky <leon@kernel.org>]

On Thu, Oct 12, 2023 at 01:29:21AM -0700, Maxim Samoylov wrote:
> This patch provides the uniform handling for RLIM_INFINITY value
> across the infiniband/rdma subsystem.
> 
> Currently in some cases the infinity constant is treated
> as an actual limit value, which could be misleading.
> 
> Let's also provide the single helper to check against process
> MEMLOCK limit while registering user memory region mappings.
> 
> Signed-off-by: Maxim Samoylov <max7255@meta.com>
> ---
> 
> v1 -> v2: rewritten commit message, rebased on recent upstream
> 
>  drivers/infiniband/core/umem.c             |  7 ++-----
>  drivers/infiniband/hw/qib/qib_user_pages.c |  7 +++----
>  drivers/infiniband/hw/usnic/usnic_uiom.c   |  6 ++----
>  drivers/infiniband/sw/siw/siw_mem.c        |  6 +++---
>  drivers/infiniband/sw/siw/siw_verbs.c      | 23 ++++++++++------------
>  include/rdma/ib_umem.h                     | 11 +++++++++++
>  6 files changed, 31 insertions(+), 29 deletions(-)

<...>

> @@ -1321,8 +1322,8 @@ struct ib_mr *siw_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
>  	struct siw_umem *umem = NULL;
>  	struct siw_ureq_reg_mr ureq;
>  	struct siw_device *sdev = to_siw_dev(pd->device);
> -
> -	unsigned long mem_limit = rlimit(RLIMIT_MEMLOCK);
> +	unsigned long num_pages =
> +		(PAGE_ALIGN(len + (start & ~PAGE_MASK))) >> PAGE_SHIFT;
>  	int rv;
>  
>  	siw_dbg_pd(pd, "start: 0x%pK, va: 0x%pK, len: %llu\n",
> @@ -1338,19 +1339,15 @@ struct ib_mr *siw_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
>  		rv = -EINVAL;
>  		goto err_out;
>  	}
> -	if (mem_limit != RLIM_INFINITY) {
> -		unsigned long num_pages =
> -			(PAGE_ALIGN(len + (start & ~PAGE_MASK))) >> PAGE_SHIFT;
> -		mem_limit >>= PAGE_SHIFT;
>  
> -		if (num_pages > mem_limit - current->mm->locked_vm) {
> -			siw_dbg_pd(pd, "pages req %lu, max %lu, lock %lu\n",
> -				   num_pages, mem_limit,
> -				   current->mm->locked_vm);
> -			rv = -ENOMEM;
> -			goto err_out;
> -		}
> +	if (!ib_umem_check_rlimit_memlock(num_pages + current->mm->locked_vm)) {
> +		siw_dbg_pd(pd, "pages req %lu, max %lu, lock %lu\n",
> +				num_pages, rlimit(RLIMIT_MEMLOCK),
> +				current->mm->locked_vm);
> +		rv = -ENOMEM;
> +		goto err_out;
>  	}

Sorry for late response, but why does this hunk exist in first place?

> +
>  	umem = siw_umem_get(start, len, ib_access_writable(rights));

This should be ib_umem_get().

Thanks