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. > > 1. https://elixir.bootlin.com/linux/latest/source/include/linux/mm.h#L219
