Re: [PATCH] IB: fix memlock limit handling code

[Leon Romanovsky <leon@kernel.org>]

On Thu, Sep 21, 2023 at 04:31:44PM +0000, Maxim Samoylov wrote:
> On 18/09/2023 14:09, Leon Romanovsky wrote:
> > On Fri, Sep 15, 2023 at 01:03:53PM -0700, Maxim Samoylov wrote:
> >> This patch fixes handling for RLIM_INFINITY value uniformly across
> >> the infiniband/rdma subsystem.
> >>
> >> Currently infinity constant is treated as actual limit
> >> value, which can trigger unexpected ENOMEM errors in
> >> corner-case configurations
> > 
> > Can you please provide an example and why these corner cases are
> > important?
> > 
> 
> Actually, I’ve come up with proposing this minor patch to avoid
> confusion I got while investigating production case with
> ib_reg_user_mr() returning ENOMEM for (presumably) no particular reason.
> 
> Along with that I came across some curious repro.
> Consider the following code:
> 
> 
> addr = mmap(... , PROT_READ | PROT_WRITE,
>              MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, ... )
> 
> /* IB objects initialisation */
> 
> while (1) {
> 
> ibv_reg_mr_iova(pd, (void*)addr, LENGTH, (uint64_t)addr,
>      IBV_ACCESS_LOCAL_WRITE|IBV_ACCESS_REMOTE_WRITE|
>      IBV_ACCESS_REMOTE_READ|IBV_ACCESS_RELAXED_ORDERING);
> 
> }
> 
> 
> This cycle can work almost eternally without triggering any errors
> - until the kernel will run out of memory or we finally bail out after
> comparison against thread memlock rlimit.
> 
> As far as I understand, this means we can continuously register the same
> memory region for a single device over and over, bloating number of
> per-device MRs. Don't know for sure if it's wrong, but
> I assume it constitutes some at least logical pitfall.
> 
> Furthermore, it also bumps per-mm VmPin counter over and over without
> increasing any other memory usage metric,
> which is probably misguiding from the memory accounting perspective.
> 
> > BTW, The patch looks good to me, just need more information in commit message.
> >
> Thanks for your quick response!
> And I apologise that my answer took so long.

Please improve your commit message, remove "fix" word as this patch
doesn't really fix anything and send v2.

Thanks

> 
> > Thanks
> > 
> > 
> >>
>