[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <SA0PR15MB391902A16F71DB916716709799DC9@SA0PR15MB3919.namprd15.prod.outlook.com>
Date: Sun, 12 Feb 2023 17:32:14 +0000
From: Bernard Metzler <BMT@...ich.ibm.com>
To: Alistair Popple <apopple@...dia.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"cgroups@...r.kernel.org" <cgroups@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jgg@...dia.com" <jgg@...dia.com>,
"jhubbard@...dia.com" <jhubbard@...dia.com>,
"tjmercier@...gle.com" <tjmercier@...gle.com>,
"hannes@...xchg.org" <hannes@...xchg.org>,
"surenb@...gle.com" <surenb@...gle.com>,
"mkoutny@...e.com" <mkoutny@...e.com>,
"daniel@...ll.ch" <daniel@...ll.ch>,
Daniel Berrange <berrange@...hat.com>,
Alex Williamson <alex.williamson@...hat.com>,
Jason Gunthorpe <jgg@...pe.ca>,
Leon Romanovsky <leon@...nel.org>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: RE: [PATCH 05/19] RMDA/siw: Convert to use vm_account
> -----Original Message-----
> From: Alistair Popple <apopple@...dia.com>
> Sent: Monday, 6 February 2023 08:48
> To: linux-mm@...ck.org; cgroups@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org; jgg@...dia.com; jhubbard@...dia.com;
> tjmercier@...gle.com; hannes@...xchg.org; surenb@...gle.com;
> mkoutny@...e.com; daniel@...ll.ch; Daniel Berrange <berrange@...hat.com>;
> Alex Williamson <alex.williamson@...hat.com>; Alistair Popple
> <apopple@...dia.com>; Bernard Metzler <BMT@...ich.ibm.com>; Jason Gunthorpe
> <jgg@...pe.ca>; Leon Romanovsky <leon@...nel.org>; linux-
> rdma@...r.kernel.org
> Subject: [EXTERNAL] [PATCH 05/19] RMDA/siw: Convert to use vm_account
>
> Convert to using a vm_account structure to account pinned memory to
> both the mm and the pins cgroup.
>
> Signed-off-by: Alistair Popple <apopple@...dia.com>
> Cc: Bernard Metzler <bmt@...ich.ibm.com>
> Cc: Jason Gunthorpe <jgg@...pe.ca>
> Cc: Leon Romanovsky <leon@...nel.org>
> Cc: linux-rdma@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> ---
> drivers/infiniband/sw/siw/siw.h | 3 ++-
> drivers/infiniband/sw/siw/siw_mem.c | 21 +++++++--------------
> drivers/infiniband/sw/siw/siw_verbs.c | 15 ---------------
> 3 files changed, 9 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw.h
> b/drivers/infiniband/sw/siw/siw.h
> index 2f3a9cd..6d4aabd 100644
> --- a/drivers/infiniband/sw/siw/siw.h
> +++ b/drivers/infiniband/sw/siw/siw.h
> @@ -13,6 +13,7 @@
> #include <crypto/hash.h>
> #include <linux/crc32.h>
> #include <linux/crc32c.h>
> +#include <linux/vm_account.h>
>
> #include <rdma/siw-abi.h>
> #include "iwarp.h"
> @@ -124,7 +125,7 @@ struct siw_umem {
> int num_pages;
> bool writable;
> u64 fp_addr; /* First page base address */
> - struct mm_struct *owning_mm;
> + struct vm_account vm_account;
> };
>
> struct siw_pble {
> diff --git a/drivers/infiniband/sw/siw/siw_mem.c
> b/drivers/infiniband/sw/siw/siw_mem.c
> index f51ab2c..be90121 100644
> --- a/drivers/infiniband/sw/siw/siw_mem.c
> +++ b/drivers/infiniband/sw/siw/siw_mem.c
> @@ -68,7 +68,6 @@ static void siw_free_plist(struct siw_page_chunk *chunk,
> int num_pages,
>
> void siw_umem_release(struct siw_umem *umem, bool dirty)
> {
> - struct mm_struct *mm_s = umem->owning_mm;
> int i, num_pages = umem->num_pages;
>
> for (i = 0; num_pages; i++) {
> @@ -79,9 +78,9 @@ void siw_umem_release(struct siw_umem *umem, bool dirty)
> kfree(umem->page_chunk[i].plist);
> num_pages -= to_free;
> }
> - atomic64_sub(umem->num_pages, &mm_s->pinned_vm);
> + vm_unaccount_pinned(&umem->vm_account, umem->num_pages);
> + vm_account_release(&umem->vm_account);
>
> - mmdrop(mm_s);
> kfree(umem->page_chunk);
> kfree(umem);
> }
> @@ -365,9 +364,7 @@ struct siw_pbl *siw_pbl_alloc(u32 num_buf)
> struct siw_umem *siw_umem_get(u64 start, u64 len, bool writable)
> {
> struct siw_umem *umem;
> - struct mm_struct *mm_s;
> u64 first_page_va;
> - unsigned long mlock_limit;
> unsigned int foll_flags = FOLL_LONGTERM;
> int num_pages, num_chunks, i, rv = 0;
>
> @@ -385,20 +382,16 @@ struct siw_umem *siw_umem_get(u64 start, u64 len,
> bool writable)
> if (!umem)
> return ERR_PTR(-ENOMEM);
>
> - mm_s = current->mm;
> - umem->owning_mm = mm_s;
> umem->writable = writable;
>
> - mmgrab(mm_s);
> + vm_account_init_current(&umem->vm_account);
>
> if (writable)
> foll_flags |= FOLL_WRITE;
>
> - mmap_read_lock(mm_s);
> + mmap_read_lock(current->mm);
>
> - mlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> -
> - if (atomic64_add_return(num_pages, &mm_s->pinned_vm) > mlock_limit) {
> + if (vm_account_pinned(&umem->vm_account, num_pages)) {
> rv = -ENOMEM;
> goto out_sem_up;
> }
> @@ -434,14 +427,14 @@ struct siw_umem *siw_umem_get(u64 start, u64 len,
> bool writable)
> }
> }
> out_sem_up:
> - mmap_read_unlock(mm_s);
> + mmap_read_unlock(current->mm);
>
> if (rv > 0)
> return umem;
>
> /* Adjust accounting for pages not pinned */
> if (num_pages)
> - atomic64_sub(num_pages, &mm_s->pinned_vm);
> + vm_unaccount_pinned(&umem->vm_account, num_pages);
>
> siw_umem_release(umem, false);
>
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
> b/drivers/infiniband/sw/siw/siw_verbs.c
> index 906fde1..8fab009 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -1321,8 +1321,6 @@ 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);
> int rv;
>
> siw_dbg_pd(pd, "start: 0x%pK, va: 0x%pK, len: %llu\n",
> @@ -1338,19 +1336,6 @@ 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;
> - }
> - }
Yes, makes sense. This double checking now and then
in siw_umem_get() was just useless. thanks!
> umem = siw_umem_get(start, len, ib_access_writable(rights));
> if (IS_ERR(umem)) {
> rv = PTR_ERR(umem);
LGTM!
Reviewed-by: Bernard Metzler <bmt@...ich.ibm.com>
> --
> git-series 0.9.1
Powered by blists - more mailing lists