Le 02/04/2019 à 22:41, Daniel Jordan a écrit :
With locked_vm now an atomic, there is no need to take mmap_sem as
writer. Delete and refactor accordingly.
Could you please detail the change ? It looks like this is not the only
change. I'm wondering what the consequences are.
Before we did:
- lock
- calculate future value
- check the future value is acceptable
- update value if future value acceptable
- return error if future value non acceptable
- unlock
Now we do:
- atomic update with future (possibly too high) value
- check the new value is acceptable
- atomic update back with older value if new value not acceptable and
return error
So if a concurrent action wants to increase locked_vm with an acceptable
step while another one has temporarily set it too high, it will now fail.
I think we should keep the previous approach and do a cmpxchg after
validating the new value.
Christophe
Signed-off-by: Daniel Jordan <daniel.m.jordan@xxxxxxxxxx>
Cc: Alexey Kardashevskiy <aik@xxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
Cc: Christoph Lameter <cl@xxxxxxxxx>
Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
Cc: Paul Mackerras <paulus@xxxxxxxxx>
Cc: <linux-mm@xxxxxxxxx>
Cc: <linuxppc-dev@xxxxxxxxxxxxxxxx>
Cc: <linux-kernel@xxxxxxxxxxxxxxx>
---
arch/powerpc/mm/mmu_context_iommu.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index 8038ac24a312..a4ef22b67c07 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -54,34 +54,29 @@ struct mm_iommu_table_group_mem_t {
static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
unsigned long npages, bool incr)
{
- long ret = 0, locked, lock_limit;
+ long ret = 0;
+ unsigned long lock_limit;
s64 locked_vm;
if (!npages)
return 0;
- down_write(&mm->mmap_sem);
- locked_vm = atomic64_read(&mm->locked_vm);
if (incr) {
- locked = locked_vm + npages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+ locked_vm = atomic64_add_return(npages, &mm->locked_vm);
+ if (locked_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
ret = -ENOMEM;
- else
- atomic64_add(npages, &mm->locked_vm);
+ atomic64_sub(npages, &mm->locked_vm);
+ }
} else {
- if (WARN_ON_ONCE(npages > locked_vm))
- npages = locked_vm;
- atomic64_sub(npages, &mm->locked_vm);
+ locked_vm = atomic64_sub_return(npages, &mm->locked_vm);
+ WARN_ON_ONCE(locked_vm < 0);
}
- pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%ld %ld/%ld\n",
- current ? current->pid : 0,
- incr ? '+' : '-',
- npages << PAGE_SHIFT,
- atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
+ pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%lu %lld/%lu\n",
+ current ? current->pid : 0, incr ? '+' : '-',
+ npages << PAGE_SHIFT, locked_vm << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
- up_write(&mm->mmap_sem);
return ret;
}
