lkml.org 
[lkml]   [2022]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v6] mm/ksm: introduce ksm_force for each process
Date
Hello.

On pátek 13. května 2022 22:32:10 CEST Andrew Morton wrote:
> On Fri, 13 May 2022 11:51:53 +0200 Oleksandr Natalenko <oleksandr@natalenko.name> wrote:
> > 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()?

Call `process_madvise()` on them again. And do that again. Regularly, with some intervals. Use some daemon for that (like [1]).

[1] https://gitlab.com/post-factum/uksmd

> > OTOH, IIUC, even if `MADV_MERGEABLE` is allowed for `process_madvise()`,
>
> Is it not?

It is not:

```
1158 static bool
1159 process_madvise_behavior_valid(int behavior)
1160 {
1161 switch (behavior) {
1162 case MADV_COLD:
1163 case MADV_PAGEOUT:
1164 case MADV_WILLNEED:
1165 return true;
1166 default:
1167 return false;
1168 }
1169 }
```

Initially, when `process_madvise()` stuff was being prepared for merging, I tried to enabled it but failed [2], and it was decided [3] to move forward without it.

[2] https://lore.kernel.org/linux-api/34f812b8-df54-eaad-5cf0-335f07da55c6@suse.cz/
[3] https://lore.kernel.org/lkml/20200623085944.cvob63vrv54fo7cs@butterfly.localdomain/

> > 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?

And still get `-ENOMEM`?

```
1191 /*
1192 * If the interval [start,end) covers some unmapped address
1193 * ranges, just ignore them, but return -ENOMEM at the end.
1194 * - different from the way of handling in mlock etc.
1195 */
```

I mean, it probably will work, and probably having the error returned is fine, but generally speaking an error value should hint that something is not being done right.

> > 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?

I do like it too. I wonder what to do with older concerns [4] [5] regarding presenting such an API.

[4] https://lore.kernel.org/lkml/20190516172452.GA2106@avx2/
[5] https://lore.kernel.org/lkml/20190515145151.GG16651@dhcp22.suse.cz/

--
Oleksandr Natalenko (post-factum)


\
 
 \ /
  Last update: 2022-05-13 22:54    [W:0.041 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site