Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 13 May 2022 13:32:10 -0700
From:   Andrew Morton <akpm@linux-foundation.org>
To:     Oleksandr Natalenko <oleksandr@natalenko.name>
Cc:     cgel.zte@gmail.com, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org, corbet@lwn.net,
        xu xin <xu.xin16@zte.com.cn>,
        Yang Yang <yang.yang29@zte.com.cn>,
        Ran Xiaokai <ran.xiaokai@zte.com.cn>,
        wangyong <wang.yong12@zte.com.cn>,
        Yunkai Zhang <zhang.yunkai@zte.com.cn>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v6] mm/ksm: introduce ksm_force for each process
Message-Id: <20220513133210.9dd0a4216bd8baaa1047562c@linux-foundation.org>
In-Reply-To: <1835064.A2aMcgg3dW@natalenko.name>
References: <20220510122242.1380536-1-xu.xin16@zte.com.cn>
        <5820954.lOV4Wx5bFT@natalenko.name>
        <20220512153753.6f999fa8f5519753d43b8fd5@linux-foundation.org>
        <1835064.A2aMcgg3dW@natalenko.name>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 13 May 2022 11:51:53 +0200 Oleksandr Natalenko <oleksandr@natalenko.name> wrote:

> Hello.
> 
> On pátek 13. května 2022 0:37:53 CEST Andrew Morton wrote:
> > On Tue, 10 May 2022 15:30:36 +0200 Oleksandr Natalenko <oleksandr@natalenko.name> wrote:
> > 
> > > > If ksm_force is set to 1, force all anonymous and 'qualified' VMAs
> > > > of this mm to be involved in KSM scanning without explicitly calling
> > > > madvise to mark VMA as MADV_MERGEABLE. But It is effective only when
> > > > the klob of /sys/kernel/mm/ksm/run is set as 1.
> > > > 
> > > > If ksm_force is set to 0, cancel the feature of ksm_force of this
> > > > process (fallback to the default state) and unmerge those merged pages
> > > > belonging to VMAs which is not madvised as MADV_MERGEABLE of this process,
> > > > but still leave MADV_MERGEABLE areas merged.
> > > 
> > > To my best knowledge, last time a forcible KSM was discussed (see threads [1], [2], [3] and probably others) it was concluded that a) procfs was a horrible interface for things like this one; and b) process_madvise() syscall was among the best suggested places to implement this (which would require a more tricky handling from userspace, but still).
> > > 
> > > So, what changed since that discussion?
> > > 
> > > P.S. For now I do it via dedicated syscall, but I'm not trying to upstream this approach.
> > 
> > Why are you patching the kernel with a new syscall rather than using
> > process_madvise()?
> 
> Because I'm not sure how to use `process_madvise()` to achieve $subj properly.
> 
> The objective is to mark all the eligible VMAs of the target task for KSM to consider them for merging.
> 
> For that, all the eligible VMAs have to be traversed.
> 
> Given `process_madvise()` has got an iovec API, this means the process that will call `process_madvise()` has to know the list of VMAs of the target process. In order to traverse them in a race-free manner the target task has to be SIGSTOP'ped or frozen, then the list of VMAs has to be obtained, then `process_madvise()` has to be called, and the the target task can continue. This is:
> 
> a) superfluous (the kernel already knows the list of VMAs of the target tasks, why proxy it through the userspace then?); and
> b) may induce more latency than needed because the target task has to be stopped to avoid races.

OK.  And what happens to new vmas that the target process creates after
the process_madvise()?

> OTOH, IIUC, even if `MADV_MERGEABLE` is allowed for `process_madvise()`,

Is it not?

> I cannot just call it like this:
> 
> ```
> iovec.iov_base = 0;
> iovec.iov_len = ~0ULL;
> process_madvise(pidfd, &iovec, 1, MADV_MERGEABLE, 0);
> ```
> 
> to cover the whole address space because iovec expects total size to be under ssize_t.
> 
> Or maybe there's no need to cover the whole address space, only the lower half of it?

Call process_madvise() twice, once for each half?

> Or maybe there's another way of doing things, and I just look stupid and do not understand how this is supposed to work?..
> 
> I'm more than happy to read your comments on this.
> 

I see the problem.  I do like the simplicity of the ksm_force concept. 
Are there alternative ideas?