Feedback-ID: i84614614:Fastmail
References: <20230123173748.1734238-1-shr@devkernel.io>
 <5844ee9f-1992-a62a-2141-3b694a1e1915@redhat.com>
User-agent: mu4e 1.6.11; emacs 28.2.50
From:   Stefan Roesch <shr@devkernel.io>
To:     David Hildenbrand <david@redhat.com>
Cc:     linux-mm@kvack.org, linux-doc@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, CGEL <cgel.zte@gmail.com>,
        Michal Hocko <mhocko@kernel.org>, Jann Horn <jannh@google.com>
Subject: Re: [RESEND RFC PATCH v1 00/20] mm: process/cgroup ksm support
Date:   Tue, 24 Jan 2023 09:37:33 -0800
In-reply-to: <5844ee9f-1992-a62a-2141-3b694a1e1915@redhat.com>
Message-ID: <qvqwbkmnj014.fsf@dev0134.prn3.facebook.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


David Hildenbrand <david@redhat.com> writes:

> On 23.01.23 18:37, Stefan Roesch wrote:
>> So far KSM can only be enabled by calling madvise for memory regions. What is
>> required to enable KSM for more workloads is to enable / disable it at the
>> process / cgroup level.
>
> Did you stumble over the proposals from last year to enable this per-process [1]
> and system-wide [2]? I remember there was also regarding enabling it
> system-wide.
>
I saw the earlier proposals. Enabling it at the system level doesn't
seem to be the right choice. You generally want to enable it at a more
fine-grained level. Therefore this enables it at the process level with
prctl.

> I'm going to point out the security aspect, and that e.g., Windows used to
> enable it system-wide before getting taught by security experts otherwise.
> Details on KSM and security aspects can be found in that thread.
>
If I'm not mistaken the security aspect exists today. When KSM is
enabled with madvise this is the same.

> Long story short: one has to be very careful with that and only enable it for
> very carefully selected worklads. Letting a workload opt-in on a VMA level is
> most probably safer than an admin blindly turning this on for random processes
> ...
>
Thats why this is enabled with prctl. Its a deliberate choice to enable
it for a process.

> Last attempts got nacked ...
>
> [1] https://lore.kernel.org/all/20220517092701.1662641-1-xu.xin16@zte.com.cn/
> [2] https://lore.kernel.org/all/20220609055658.703472-1-xu.xin16@zte.com.cn/
>
My understanding is that there were problems with the patch and how it
exposed KSM. The other objection was the enable-all configuration
option.

>> 1. New options for prctl system command
>> This patch series adds two new options to the prctl system call. The first
>> one allows to enable KSM at the process level and the second one to query the
>> setting.
>> The setting will be inherited by child processes.
>> With the above setting, KSM can be enabled for the seed process of a cgroup
>> and all processes in the cgroup will inherit the setting.
>> 2. Changes to KSM processing
>> When KSM is enabled at the process level, the KSM code will iterate over all
>> the VMA's and enable KSM for the eligible VMA's.
>> When forking a process that has KSM enabled, the setting will be inherited by
>> the new child process.
>> In addition when KSM is disabled for a process, KSM will be disabled for the
>> VMA's where KSM has been enabled.
>> 3. Add tracepoints to KSM
>> Currently KSM has no tracepoints. This adds tracepoints to the key KSM functions
>> to make it easier to debug KSM.
>> 4. Add general_profit metric
>> The general_profit metric of KSM is specified in the documentation, but not
>> calculated. This adds the general profit metric to /sys/kernel/debug/mm/ksm.
>> 5. Add more metrics to ksm_stat
>> This adds the process profit and ksm type metric to /proc/<pid>/ksm_stat.
>> 6. Add more tests to ksm_tests
>> This adds an option to specify the merge type to the ksm_tests. This allows to
>> test madvise and prctl KSM. It also adds a new option to query if prctl KSM has
>> been enabled. It adds a fork test to verify that the KSM process setting is
>> inherited by client processes.
>> Stefan Roesch (20):
>>    mm: add new flag to enable ksm per process
>>    mm: add flag to __ksm_enter
>>    mm: add flag to __ksm_exit call
>>    mm: invoke madvise for all vmas in scan_get_next_rmap_item
>>    mm: support disabling of ksm for a process
>>    mm: add new prctl option to get and set ksm for a process
>>    mm: add tracepoints to ksm
>>    mm: split off pages_volatile function
>>    mm: expose general_profit metric
>>    docs: document general_profit sysfs knob
>>    mm: calculate ksm process profit metric
>>    mm: add ksm_merge_type() function
>>    mm: expose ksm process profit metric in ksm_stat
>>    mm: expose ksm merge type in ksm_stat
>>    docs: document new procfs ksm knobs
>>    tools: add new prctl flags to prctl in tools dir
>>    selftests/vm: add KSM prctl merge test
>>    selftests/vm: add KSM get merge type test
>>    selftests/vm: add KSM fork test
>>    selftests/vm: add two functions for debugging merge outcome
>>   Documentation/ABI/testing/sysfs-kernel-mm-ksm |   8 +
>>   Documentation/admin-guide/mm/ksm.rst          |   8 +-
>>   MAINTAINERS                                   |   1 +
>>   fs/proc/base.c                                |   5 +
>>   include/linux/ksm.h                           |  19 +-
>>   include/linux/sched/coredump.h                |   1 +
>>   include/trace/events/ksm.h                    | 257 ++++++++++++++++++
>>   include/uapi/linux/prctl.h                    |   2 +
>>   kernel/sys.c                                  |  29 ++
>>   mm/ksm.c                                      | 134 ++++++++-
>>   tools/include/uapi/linux/prctl.h              |   2 +
>>   tools/testing/selftests/vm/Makefile           |   3 +-
>>   tools/testing/selftests/vm/ksm_tests.c        | 254 ++++++++++++++---
>>   13 files changed, 665 insertions(+), 58 deletions(-)
>>   create mode 100644 include/trace/events/ksm.h
>> base-commit: c1649ec55708ae42091a2f1bca1ab49ecd722d55