Re: [PATCH V3] KSM: allow dedup all tasks memory

[Timofey Titovets]

ÐÑ, 13 ÐÐÑÐ. 2018 Ð. Ð 22:17, Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>:
>
> On 18-11-13 21:54:13, Timofey Titovets wrote:
> > ÐÑ, 13 ÐÐÑÐ. 2018 Ð. Ð 21:35, Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>:
> > >
> > > On 18-11-13 21:17:42, Timofey Titovets wrote:
> > > > ÐÑ, 13 ÐÐÑÐ. 2018 Ð. Ð 20:59, Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>:
> > > > >
> > > > > On 18-11-13 15:23:50, Oleksandr Natalenko wrote:
> > > > > > Hi.
> > > > > >
> > > > > > > Yep. However, so far, it requires an application to explicitly opt in
> > > > > > > to this behavior, so it's not all that bad. Your patch would remove
> > > > > > > the requirement for application opt-in, which, in my opinion, makes
> > > > > > > this way worse and reduces the number of applications for which this
> > > > > > > is acceptable.
> > > > > >
> > > > > > The default is to maintain the old behaviour, so unless the explicit
> > > > > > decision is made by the administrator, no extra risk is imposed.
> > > > >
> > > > > The new interface would be more tolerable if it honored MADV_UNMERGEABLE:
> > > > >
> > > > > KSM default on: merge everything except when MADV_UNMERGEABLE is
> > > > > excplicitly set.
> > > > >
> > > > > KSM default off: merge only when MADV_MERGEABLE is set.
> > > > >
> > > > > The proposed change won't honor MADV_UNMERGEABLE, meaning that
> > > > > application programmers won't have a way to prevent sensitive data to be
> > > > > every merged. So, I think, we should keep allow an explicit opt-out
> > > > > option for applications.
> > > > >
> > > >
> > > > We just did not have VM/Madvise flag for that currently.
> > > > Same as THP.
> > > > Because all logic written with assumption, what we have exactly 2 states.
> > > > Allow/Disallow (More like not allow).
> > > >
> > > > And if we try to add, that must be something like:
> > > > MADV_FORBID_* to disallow something completely.
> > >
> > > No need to add new user flag MADV_FORBID, we should keep MADV_MERGEABLE
> > > and MADV_UNMERGEABLE, but make them work so when MADV_UNMERGEABLE is
> > > set, memory is indeed becomes always unmergeable regardless of ksm mode
> > > of operation.
> > >
> > > To do the above in ksm_madvise(), a new state should be added, for example
> > > instead of:
> > >
> > > case MADV_UNMERGEABLE:
> > >         *vm_flags &= ~VM_MERGEABLE;
> > >
> > > A new flag should be used:
> > >         *vm_flags |= VM_UNMERGEABLE;
> > >
> > > I think, without honoring MADV_UNMERGEABLE correctly, this patch won't
> > > be accepted.
> > >
> > > Pasha
> > >
> >
> > That must work, but we out of bit space in vm_flags [1].
> > i.e. first 32 bits already defined, and other only accessible only on
> > 64-bit machines.
>
> So, grow vm_flags_t to 64-bit, or enable this feature on 64-bit only.

With all due respect to you, for that type of things we need
mm maintainer opinion.

I just don't want get situation, where after touch of other subsystems,
maintainer will just refuse that work by some reason.

i.e. writing patches for upstream (from my point of view),
is more art of communication and making resulte code acceptable by community.
Because any code which written correctly from engineering point of view,
can be easy refused, just because someone not found it useful.

Thanks.

> >
> > 1. https://elixir.bootlin.com/linux/latest/source/include/linux/mm.h#L219