Re: [PATCH] mm/ksm: introduce ksm_enabled for each processg

[CGEL <cgel.zte@gmail.com>]

On Tue, May 24, 2022 at 11:04:40AM +0200, Michal Hocko wrote:
> On Tue 24-05-22 08:52:02, CGEL wrote:
> > On Thu, May 19, 2022 at 09:39:57AM +0200, Michal Hocko wrote:
> > > On Thu 19-05-22 06:35:03, CGEL wrote:
> > > > On Wed, May 18, 2022 at 02:14:28PM +0200, Michal Hocko wrote:
> > > > > On Wed 18-05-22 07:40:30, CGEL wrote:
> > > > > [...]
> > > > > > 2. process_madvise is still a kind of madvise. processs_madvise from
> > > > > > another process overrides the intention of origin app code ifself that
> > > > > > also calls madvise, which is unrecoverable. For example, if a process "A"
> > > > > > which madvises just one part of VMAs (not all) as MERGEABLE run on the OS
> > > > > > already, meanwhile, if another process which doesn't know the information
> > > > > > of "A" 's MERGEABLE areas, then call process_madvise to advise all VMAs of
> > > > > > "A" as MERGEABLE, the original MERGEABLE information of "A" calling madivse
> > > > > > is erasured permanently.
> > > > > 
> > > > > I do not really follow. How is this any different from an external
> > > > > process modifying the process wide policy via the proc or any other
> > > > > interface?
> > > > 
> > > > In this patch, you can see that we didn't modify the flag of any VMA of
> > > > the target process, which is different from process_madvise. So it is
> > > > easy to keep the original MERGEABLE information of the target process
> > > > when we turn back to the default state from the state "always".
> > > 
> > > This means that /proc/<pid>/smaps doesn't show the real state, right?
> > 
> > Maybe we can add extra information of KSM forcible state in /proc/<pid>/smaps
> > like THPeligible. 
> 
> That information is already printed and I do not think that adding
> another flag or whatever would make the situation much more clear.
> 
> > Really, Michal, I think it again, 'process_ madvise' is really not good. In
> > addition to some shortcomings I said before, If new vmas of the target process
> > are created after the external process calls process_madvise(), then we have to
> > call `process_madvise()` on them again, over and over again, regularly, just like
> > Oleksandr said [1].
> 
> I can see that this is not the most convenient way but so far I haven't
> really heard any arguments that this would be impossible.
> 
> Look, I am not claiming that process_madvise is the only way to achieve
> the goal. I really do not like the proc based interface because it is
> rather adhoc and limited. We have other means to set a process wide
> property and I do not see any strong arguments agaist prctl.
> 

I can agree with you that proc is adhoc and limit. Use prctl extension
is probably better, but the problem is that it can't control external
process directly.

> But more importantly I haven't really seen any serious analysis whether
> per-process (resp. per MM) property is even a desirable interface.
> Especially in the current form when opting out for certain VMAs is not
> possible.

I think the reasons of using per-process (resp. per MM) property are as
follows:

The KSM mandatory attribute is set for the entire mm space rather than
some VMAs. Its system is to allow all eligible VMAs of the entire mm to
participate in KSM. Although marking all VMAs as mergeble can achieve the
same purpose, the concept is different:

From another perspective, for example, the rule of a company is to hold
a morning meeting at 9:30 a.m., but one day, the local law stipulates
that it is illegal to go to work before 10 o'clock, then this rule of
the company have to be covered and invalid. Here, 'mm->ksm_ enabled' is
analogous to local laws, while the company's rule is analogous to VMA
-> flag. One day, after the local law is repealed, the company's rule
can still be restored.

>  
> 
> -- 
> Michal Hocko
> SUSE Labs