David Hildenbrand <david@xxxxxxxxxx> writes:
> Let's unmerge any KSM pages when setting PR_SET_MEMORY_MERGE=0, and clear
> the VM_MERGEABLE flag from all VMAs -- just like KSM would. Of course,
> only do that if we previously set PR_SET_MEMORY_MERGE=1.
>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> include/linux/ksm.h | 1 +
> kernel/sys.c | 7 +------
> mm/ksm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/ksm.h b/include/linux/ksm.h
> index 590934bdddcf..7108bc65dc2a 100644
> --- a/include/linux/ksm.h
> +++ b/include/linux/ksm.h
> @@ -21,6 +21,7 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
>
> void ksm_add_vma(struct vm_area_struct *vma);
> int ksm_enable_merge_any(struct mm_struct *mm);
> +int ksm_disable_merge_any(struct mm_struct *mm);
>
> int __ksm_enter(struct mm_struct *mm);
> void __ksm_exit(struct mm_struct *mm);
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 72cdb16e2636..3436376667d7 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2698,12 +2698,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> if (arg2) {
> error = ksm_enable_merge_any(me->mm);
> } else {
> - /*
> - * TODO: we might want disable KSM on all VMAs and
> - * trigger unsharing to completely disable KSM.
> - */
> - clear_bit(MMF_VM_MERGE_ANY, &me->mm->flags);
> - error = 0;
> + error = ksm_disable_merge_any(me->mm);
> }
>
nit:
can we do:
if (arg2)
error = ksm_enable_merge_any(me->mm);
else
error = ksm_disable_merge_any(me->mm);
mmap_write_unlock(me->mm);
break;
> diff --git a/mm/ksm.c b/mm/ksm.c
> index a959e8925413..813f7fbc1832 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -2520,6 +2520,22 @@ static void __ksm_add_vma(struct vm_area_struct *vma)
> vm_flags_set(vma, VM_MERGEABLE);
> }
>
> +static int __ksm_del_vma(struct vm_area_struct *vma)
> +{
> + int err;
> +
> + if (!(vma->vm_flags & VM_MERGEABLE))
> + return 0;
> +
> + if (vma->anon_vma) {
> + err = unmerge_ksm_pages(vma, vma->vm_start, vma->vm_end);
> + if (err)
> + return err;
> + }
> +
> + vm_flags_clear(vma, VM_MERGEABLE);
> + return 0;
> +}
> /**
> * ksm_add_vma - Mark vma as mergeable if compatible
> *
> @@ -2542,6 +2558,20 @@ static void ksm_add_vmas(struct mm_struct *mm)
> __ksm_add_vma(vma);
> }
>
> +static int ksm_del_vmas(struct mm_struct *mm)
> +{
> + struct vm_area_struct *vma;
> + int err;
> +
> + VMA_ITERATOR(vmi, mm, 0);
> + for_each_vma(vmi, vma) {
> + err = __ksm_del_vma(vma);
> + if (err)
> + return err;
> + }
> + return 0;
> +}
> +
> /**
> * ksm_enable_merge_any - Add mm to mm ksm list and enable merging on all
> * compatible VMA's
> @@ -2569,6 +2599,23 @@ int ksm_enable_merge_any(struct mm_struct *mm)
> return 0;
> }
>
> +int ksm_disable_merge_any(struct mm_struct *mm)
>
I understand we want to keep the name "symmetric" with
ksm_enable_merge_any, but it also unmerges the ksm pages. Do we want to
reflect that in the function name?
Can we add a comment for the function?
> +{
> + int err;
> +
> + if (!test_bit(MMF_VM_MERGE_ANY, &mm->flags))
> + return 0;
> +
> + err = ksm_del_vmas(mm);
> + if (err) {
> + ksm_add_vmas(mm);
> + return err;
> + }
> +
> + clear_bit(MMF_VM_MERGE_ANY, &mm->flags);
>
We only clear the MMF_VM_MERGE_ANY flag if there are no errors. Is this
what we want? This means that if the process creates new memory regions
they would still be marked as mergeable.
> + return 0;
> +}
> +
> int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
> unsigned long end, int advice, unsigned long *vm_flags)
> {
