David Hildenbrand <david@xxxxxxxxxx> writes:
> Thanks!
>
> In general,
>
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>
>
> Two nits below, after staring at some other prctl implementations.
>
>> +#define PR_SET_MEMORY_MERGE 67
>> +#define PR_GET_MEMORY_MERGE 68
>> #endif /* _LINUX_PRCTL_H */
>> diff --git a/kernel/sys.c b/kernel/sys.c
>> index 495cd87d9bf4..8c2e50edeb18 100644
>> --- a/kernel/sys.c
>> +++ b/kernel/sys.c
>> @@ -15,6 +15,7 @@
>> #include <linux/highuid.h>
>> #include <linux/fs.h>
>> #include <linux/kmod.h>
>> +#include <linux/ksm.h>
>> #include <linux/perf_event.h>
>> #include <linux/resource.h>
>> #include <linux/kernel.h>
>> @@ -2661,6 +2662,30 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>> case PR_SET_VMA:
>> error = prctl_set_vma(arg2, arg3, arg4, arg5);
>> break;
>> +#ifdef CONFIG_KSM
>> + case PR_SET_MEMORY_MERGE:
>
> Looking at some other code (PR_SET_NO_NEW_PRIVS/ PR_SET_THP_DISABLE) I wonder if
> we also want
>
> if (arg3 || arg4 || arg5)
> return -EINVAL;
>
I added the above check. It requires that we always specify all
parameters in the test programs. I also changed them accordingly.
> For PR_GET_MEMORY_MERGE it looks good already.
>
>> + if (mmap_write_lock_killable(me->mm))
>> + return -EINTR;
>> +
>> + 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;
>> + }
>> + mmap_write_unlock(me->mm);
>> + break;
>> + case PR_GET_MEMORY_MERGE:
>> + if (arg2 || arg3 || arg4 || arg5)
>> + return -EINVAL;
>> +
>> + error = !!test_bit(MMF_VM_MERGE_ANY, &me->mm->flags);
>> + break;
>> +#endif
>> default:
>> error = -EINVAL;
>> break;
>
> [...]
>
>> +/**
>> + * ksm_enable_merge_any - Add mm to mm ksm list and enable merging on all
>> + * compatible VMA's
>> + *
>> + * @mm: Pointer to mm
>> + *
>> + * Returns 0 on success, otherwise error code
>> + */
>> +int ksm_enable_merge_any(struct mm_struct *mm)
>> +{
>> + int err;
>> +
>> + if (test_bit(MMF_VM_MERGE_ANY, &mm->flags))
>> + return -EINVAL;
>
>
> I'm curious, why is enabling the prctl() supposed to fail if already enabled?
> (it would not fail if disabling and already disabled)
>
I changed that to not return an error in that case.
>
> For example, PR_SET_THP_DISABLE/PR_SET_NO_NEW_PRIVS doesn't fail if already set.
