Re: [PATCH v3] mm: add account_locked_vm utility function
On Wed, 29 May 2019 16:50:19 -0400
Daniel Jordan wrote:
> locked_vm accounting is done roughly the same way in five places, so
> unify them in a helper.
>
> Include the helper's caller in the debug print to distinguish between
> callsites.
>
> Error codes stay the same, so user-visible behavior does too. The one
> exception is that the -EPERM case in tce_account_locked_vm is removed
> because Alexey has never seen it triggered.
>
> Signed-off-by: Daniel Jordan
> Tested-by: Alexey Kardashevskiy
> Cc: Alan Tull
> Cc: Alex Williamson
> Cc: Andrew Morton
> Cc: Benjamin Herrenschmidt
> Cc: Christoph Lameter
> Cc: Christophe Leroy
> Cc: Davidlohr Bueso
> Cc: Ira Weiny
> Cc: Jason Gunthorpe
> Cc: Mark Rutland
> Cc: Michael Ellerman
> Cc: Moritz Fischer
> Cc: Paul Mackerras
> Cc: Steve Sistare
> Cc: Wu Hao
> Cc: linux...@kvack.org
> Cc: k...@vger.kernel.org
> Cc: kvm-...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-f...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v3:
> - uninline account_locked_vm (Andrew)
> - fix doc comment (Ira)
> - retain down_write_killable in vfio type1 (Alex)
> - leave Alexey's T-b since the code is the same aside from uninlining
> - sanity tested with vfio type1, sanity-built on ppc
>
> arch/powerpc/kvm/book3s_64_vio.c | 44 ++--
> arch/powerpc/mm/book3s64/iommu_api.c | 41 ++-
> drivers/fpga/dfl-afu-dma-region.c| 53 ++--
> drivers/vfio/vfio_iommu_spapr_tce.c | 54 ++--
> drivers/vfio/vfio_iommu_type1.c | 17 +--
> include/linux/mm.h | 4 ++
> mm/util.c| 75
> 7 files changed, 98 insertions(+), 190 deletions(-)
I tend to prefer adding a negative rather than converting to absolute
and passing a bool for inc/dec, but it all seems equivalent, so for
vfio parts
Acked-by: Alex Williamson
>
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c
> b/arch/powerpc/kvm/book3s_64_vio.c
> index 66270e07449a..768b645c7edf 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -56,43 +57,6 @@ static unsigned long kvmppc_stt_pages(unsigned long
> tce_pages)
> return tce_pages + ALIGN(stt_bytes, PAGE_SIZE) / PAGE_SIZE;
> }
>
> -static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
> -{
> - long ret = 0;
> -
> - if (!current || !current->mm)
> - return ret; /* process exited */
> -
> - down_write(>mm->mmap_sem);
> -
> - if (inc) {
> - unsigned long locked, lock_limit;
> -
> - locked = current->mm->locked_vm + stt_pages;
> - lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> - if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> - ret = -ENOMEM;
> - else
> - current->mm->locked_vm += stt_pages;
> - } else {
> - if (WARN_ON_ONCE(stt_pages > current->mm->locked_vm))
> - stt_pages = current->mm->locked_vm;
> -
> - current->mm->locked_vm -= stt_pages;
> - }
> -
> - pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%ld %ld/%ld%s\n", current->pid,
> - inc ? '+' : '-',
> - stt_pages << PAGE_SHIFT,
> - current->mm->locked_vm << PAGE_SHIFT,
> - rlimit(RLIMIT_MEMLOCK),
> - ret ? " - exceeded" : "");
> -
> - up_write(>mm->mmap_sem);
> -
> - return ret;
> -}
> -
> static void kvm_spapr_tce_iommu_table_free(struct rcu_head *head)
> {
> struct kvmppc_spapr_tce_iommu_table *stit = container_of(head,
> @@ -302,7 +266,7 @@ static int kvm_spapr_tce_release(struct inode *inode,
> struct file *filp)
>
> kvm_put_kvm(stt->kvm);
>
> - kvmppc_account_memlimit(
> + account_locked_vm(current->mm,
> kvmppc_stt_pages(kvmppc_tce_pages(stt->size)), false);
> call_rcu(>rcu, release_spapr_tce_table);
>
> @@ -327,7 +291,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
> return -EINVAL;
>
> npages = kvmppc_tce_pages(size);
> - ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
> + ret = account_locked_vm(current->mm, kvmppc_stt_pages(npages), true);
> if (ret)
> return ret;
>
> @@ -373,7 +337,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
>
> kfree(stt);
> fail_acct:
> - kvmppc_account_memlimit(kvmppc_stt_pages(npages), false);
> + account_locked_vm(current->mm, kvmppc_stt_pages(npages), false);
> return ret;
> }
>
> diff --git a/arch/powerpc/mm/book3s64/iommu_api.c
> b/arch/powerpc/mm/book3s64/iommu_api.c
> index 5c521f3924a5..18d22eec0ebd 100644
> --- a/arch/powerpc/mm/book3s64/iommu_api.c
> +++
[PATCH v3] mm: add account_locked_vm utility function
locked_vm accounting is done roughly the same way in five places, so
unify them in a helper.
Include the helper's caller in the debug print to distinguish between
callsites.
Error codes stay the same, so user-visible behavior does too. The one
exception is that the -EPERM case in tce_account_locked_vm is removed
because Alexey has never seen it triggered.
Signed-off-by: Daniel Jordan
Tested-by: Alexey Kardashevskiy
Cc: Alan Tull
Cc: Alex Williamson
Cc: Andrew Morton
Cc: Benjamin Herrenschmidt
Cc: Christoph Lameter
Cc: Christophe Leroy
Cc: Davidlohr Bueso
Cc: Ira Weiny
Cc: Jason Gunthorpe
Cc: Mark Rutland
Cc: Michael Ellerman
Cc: Moritz Fischer
Cc: Paul Mackerras
Cc: Steve Sistare
Cc: Wu Hao
Cc: linux...@kvack.org
Cc: k...@vger.kernel.org
Cc: kvm-...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-f...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
v3:
- uninline account_locked_vm (Andrew)
- fix doc comment (Ira)
- retain down_write_killable in vfio type1 (Alex)
- leave Alexey's T-b since the code is the same aside from uninlining
- sanity tested with vfio type1, sanity-built on ppc
arch/powerpc/kvm/book3s_64_vio.c | 44 ++--
arch/powerpc/mm/book3s64/iommu_api.c | 41 ++-
drivers/fpga/dfl-afu-dma-region.c| 53 ++--
drivers/vfio/vfio_iommu_spapr_tce.c | 54 ++--
drivers/vfio/vfio_iommu_type1.c | 17 +--
include/linux/mm.h | 4 ++
mm/util.c| 75
7 files changed, 98 insertions(+), 190 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 66270e07449a..768b645c7edf 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
@@ -56,43 +57,6 @@ static unsigned long kvmppc_stt_pages(unsigned long
tce_pages)
return tce_pages + ALIGN(stt_bytes, PAGE_SIZE) / PAGE_SIZE;
}
-static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
-{
- long ret = 0;
-
- if (!current || !current->mm)
- return ret; /* process exited */
-
- down_write(>mm->mmap_sem);
-
- if (inc) {
- unsigned long locked, lock_limit;
-
- locked = current->mm->locked_vm + stt_pages;
- lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- if (locked > lock_limit && !capable(CAP_IPC_LOCK))
- ret = -ENOMEM;
- else
- current->mm->locked_vm += stt_pages;
- } else {
- if (WARN_ON_ONCE(stt_pages > current->mm->locked_vm))
- stt_pages = current->mm->locked_vm;
-
- current->mm->locked_vm -= stt_pages;
- }
-
- pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%ld %ld/%ld%s\n", current->pid,
- inc ? '+' : '-',
- stt_pages << PAGE_SHIFT,
- current->mm->locked_vm << PAGE_SHIFT,
- rlimit(RLIMIT_MEMLOCK),
- ret ? " - exceeded" : "");
-
- up_write(>mm->mmap_sem);
-
- return ret;
-}
-
static void kvm_spapr_tce_iommu_table_free(struct rcu_head *head)
{
struct kvmppc_spapr_tce_iommu_table *stit = container_of(head,
@@ -302,7 +266,7 @@ static int kvm_spapr_tce_release(struct inode *inode,
struct file *filp)
kvm_put_kvm(stt->kvm);
- kvmppc_account_memlimit(
+ account_locked_vm(current->mm,
kvmppc_stt_pages(kvmppc_tce_pages(stt->size)), false);
call_rcu(>rcu, release_spapr_tce_table);
@@ -327,7 +291,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
return -EINVAL;
npages = kvmppc_tce_pages(size);
- ret = kvmppc_account_memlimit(kvmppc_stt_pages(npages), true);
+ ret = account_locked_vm(current->mm, kvmppc_stt_pages(npages), true);
if (ret)
return ret;
@@ -373,7 +337,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
kfree(stt);
fail_acct:
- kvmppc_account_memlimit(kvmppc_stt_pages(npages), false);
+ account_locked_vm(current->mm, kvmppc_stt_pages(npages), false);
return ret;
}
diff --git a/arch/powerpc/mm/book3s64/iommu_api.c
b/arch/powerpc/mm/book3s64/iommu_api.c
index 5c521f3924a5..18d22eec0ebd 100644
--- a/arch/powerpc/mm/book3s64/iommu_api.c
+++ b/arch/powerpc/mm/book3s64/iommu_api.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -51,40 +52,6 @@ struct mm_iommu_table_group_mem_t {
u64 dev_hpa;/* Device memory base address */
};
-static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
- unsigned long npages, bool incr)
-{
- long ret = 0, locked, lock_limit;
-
- if (!npages)
- return 0;
-
-
