Date:   Wed, 25 Jan 2023 14:01:27 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     David Hildenbrand <david@redhat.com>
Cc:     Stefan Roesch <shr@devkernel.io>, 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>,
        Jann Horn <jannh@google.com>
Subject: Re: [RESEND RFC PATCH v1 00/20] mm: process/cgroup ksm support
Message-ID: <Y9EoJ0jlXMeuJzuY@dhcp22.suse.cz>
References: <20230123173748.1734238-1-shr@devkernel.io>
 <5844ee9f-1992-a62a-2141-3b694a1e1915@redhat.com>
 <qvqwbkmnj014.fsf@dev0134.prn3.facebook.com>
 <a391e98c-88af-886c-0426-c41c9980afa1@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a391e98c-88af-886c-0426-c41c9980afa1@redhat.com>
Precedence: bulk

On Tue 24-01-23 19:01:49, David Hildenbrand wrote:
> [...]
> 
> > > 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.
> 
> Yes, and we mostly only use it for virtual machines -- and to be precise,
> guest memory only -- where it has to be enabled explicitly on a well
> documented basis ...
> 
> Impossible for an admin to force it on other parts of the hypervisor process
> that might be more security sensitive. Or on other arbitrary applications,
> for now.
> 
> > 
> > > 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
> > > ... >>
> [...]
> 
> > > 
> > > [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.
> 
> I don't remember all the discussions, but one concern was how to handle
> processes that deliberately want to disable it on some parts of memory.
> 
> Anyhow, I cc'ed the relevant parties already.

Thanks! I do not remember all the details from the past discussion
except it was /proc and later global setting. Neither really were great
choices. We have discussed pidfd_madvise and prctl IIRC. For the latter
there was no real argument about when it is desirable to apply merging
on all existing vmas (e.g. is it really desirable on stack/brk or malloc
arenas or would user need to opt out for those).

I have read through your cover letter and it talks about the interface
but it doesn't really talk about usecases and how they are supposed to
use this feature - except the prctl based flag gets inherited. So could
you elaborate some more about those usecases please?
-- 
Michal Hocko
SUSE Labs