| Message ID | 03ed2d166826cd7055810c66a175e20fa2153c52.1674538665.git-series.apopple@nvidia.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [RFC,01/19] mm: Introduce vm_account | expand |
On Tue, Jan 24, 2023 at 04:42:35PM +1100, Alistair Popple wrote: > diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c > index c301b3b..250276e 100644 > --- a/drivers/infiniband/hw/usnic/usnic_uiom.c > +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c > @@ -89,8 +89,6 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, > struct page **page_list; > struct scatterlist *sg; > struct usnic_uiom_chunk *chunk; > - unsigned long locked; > - unsigned long lock_limit; > unsigned long cur_base; > unsigned long npages; > int ret; > @@ -123,10 +121,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, > uiomr->owning_mm = mm = current->mm; > mmap_read_lock(mm); > > - locked = atomic64_add_return(npages, ¤t->mm->pinned_vm); > - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; > - > - if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) { > + vm_account_init_current(&uiomr->vm_account); > + if (vm_account_pinned(&uiomr->vm_account, npages)) { > ret = -ENOMEM; > goto out; > } Is this error handling right? This driver tried to avoid the race by using atomic64_add_return() but it means that the out label undoes the add: > @@ -178,7 +174,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, > out: > if (ret < 0) { > usnic_uiom_put_pages(chunk_list, 0); > - atomic64_sub(npages, ¤t->mm->pinned_vm); Here > + vm_unaccount_pinned(&uiomr->vm_account, npages); > + vm_account_release(&uiomr->vm_account); But with the new API we shouldn't call vm_unaccount_pinned() if vm_account_pinned() doesn't succeed? Jason
Jason Gunthorpe <jgg@nvidia.com> writes: > On Tue, Jan 24, 2023 at 04:42:35PM +1100, Alistair Popple wrote: >> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c >> index c301b3b..250276e 100644 >> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c >> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c >> @@ -89,8 +89,6 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, >> struct page **page_list; >> struct scatterlist *sg; >> struct usnic_uiom_chunk *chunk; >> - unsigned long locked; >> - unsigned long lock_limit; >> unsigned long cur_base; >> unsigned long npages; >> int ret; >> @@ -123,10 +121,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, >> uiomr->owning_mm = mm = current->mm; >> mmap_read_lock(mm); >> >> - locked = atomic64_add_return(npages, ¤t->mm->pinned_vm); >> - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; >> - >> - if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) { >> + vm_account_init_current(&uiomr->vm_account); >> + if (vm_account_pinned(&uiomr->vm_account, npages)) { >> ret = -ENOMEM; >> goto out; >> } > > Is this error handling right? This driver tried to avoid the race by > using atomic64_add_return() but it means that the out label undoes the add: > > >> @@ -178,7 +174,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, >> out: >> if (ret < 0) { >> usnic_uiom_put_pages(chunk_list, 0); >> - atomic64_sub(npages, ¤t->mm->pinned_vm); > > Here > >> + vm_unaccount_pinned(&uiomr->vm_account, npages); >> + vm_account_release(&uiomr->vm_account); > > But with the new API we shouldn't call vm_unaccount_pinned() if > vm_account_pinned() doesn't succeed? Good point. Will add the following fix: @@ -123,6 +123,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, vm_account_init_current(&uiomr->vm_account); if (vm_account_pinned(&uiomr->vm_account, npages)) { + npages = 0; ret = -ENOMEM; goto out; } > > Jason
diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c index c301b3b..250276e 100644 --- a/drivers/infiniband/hw/usnic/usnic_uiom.c +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c @@ -89,8 +89,6 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, struct page **page_list; struct scatterlist *sg; struct usnic_uiom_chunk *chunk; - unsigned long locked; - unsigned long lock_limit; unsigned long cur_base; unsigned long npages; int ret; @@ -123,10 +121,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, uiomr->owning_mm = mm = current->mm; mmap_read_lock(mm); - locked = atomic64_add_return(npages, ¤t->mm->pinned_vm); - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT; - - if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) { + vm_account_init_current(&uiomr->vm_account); + if (vm_account_pinned(&uiomr->vm_account, npages)) { ret = -ENOMEM; goto out; } @@ -178,7 +174,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable, out: if (ret < 0) { usnic_uiom_put_pages(chunk_list, 0); - atomic64_sub(npages, ¤t->mm->pinned_vm); + vm_unaccount_pinned(&uiomr->vm_account, npages); + vm_account_release(&uiomr->vm_account); } else mmgrab(uiomr->owning_mm); @@ -430,7 +427,7 @@ void usnic_uiom_reg_release(struct usnic_uiom_reg *uiomr) { __usnic_uiom_reg_release(uiomr->pd, uiomr, 1); - atomic64_sub(usnic_uiom_num_pages(uiomr), &uiomr->owning_mm->pinned_vm); + vm_unaccount_pinned(&uiomr->vm_account, usnic_uiom_num_pages(uiomr)); __usnic_uiom_release_tail(uiomr); } diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.h b/drivers/infiniband/hw/usnic/usnic_uiom.h index 5a9acf9..5c296a7 100644 --- a/drivers/infiniband/hw/usnic/usnic_uiom.h +++ b/drivers/infiniband/hw/usnic/usnic_uiom.h @@ -72,6 +72,7 @@ struct usnic_uiom_reg { struct list_head chunk_list; struct work_struct work; struct mm_struct *owning_mm; + struct vm_account vm_account; }; struct usnic_uiom_chunk {
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@nvidia.com> Cc: Christian Benvenuti <benve@cisco.com> Cc: Nelson Escobar <neescoba@cisco.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Leon Romanovsky <leon@kernel.org> Cc: linux-rdma@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- drivers/infiniband/hw/usnic/usnic_uiom.c | 13 +++++-------- drivers/infiniband/hw/usnic/usnic_uiom.h | 1 + 2 files changed, 6 insertions(+), 8 deletions(-)