[PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

By default, KSM works only on memory that is marked by madvise(). And the
only way to get around that is to either:

  * use LD_PRELOAD; or
  * patch the kernel with something like UKSM or PKSM.

Instead, lets implement a so-called "always" mode, which allows marking
VMAs as mergeable on do_anonymous_page() call automatically.

The submission introduces a new sysctl knob as well as kernel cmdline option
to control which mode to use. The default mode is to maintain old
(madvise-based) behaviour.

Due to security concerns, this submission also introduces VM_UNMERGEABLE
vmaflag for apps to explicitly opt out of automerging. Because of adding
a new vmaflag, the whole work is available for 64-bit architectures only.

This patchset is based on earlier Timofey's submission [1], but it doesn't
use dedicated kthread to walk through the list of tasks/VMAs.

For my laptop it saves up to 300 MiB of RAM for usual workflow (browser,
terminal, player, chats etc). Timofey's submission also mentions
containerised workload that benefits from automerging too.

Open questions:

  * once "always" mode is activated, should re-scan of all VMAs be
    triggered to find eligible ones for automerging?

Thanks.

[1] https://lore.kernel.org/patchwork/patch/1012142/

Oleksandr Natalenko (4):
  mm/ksm: introduce ksm_enter() helper
  mm/ksm: introduce VM_UNMERGEABLE
  mm/ksm: allow anonymous memory automerging
  mm/ksm: add automerging documentation

 .../admin-guide/kernel-parameters.txt         |   7 +
 Documentation/admin-guide/mm/ksm.rst          |   7 +
 fs/proc/task_mmu.c                            |   3 +
 include/linux/ksm.h                           |   5 +
 include/linux/mm.h                            |   6 +
 include/trace/events/mmflags.h                |   7 +
 mm/ksm.c                                      | 142 ++++++++++++++----
 mm/memory.c                                   |   6 +
 8 files changed, 157 insertions(+), 26 deletions(-)

-- 
2.21.0

[PATCH RFC 1/4] mm/ksm: introduce ksm_enter() helper

[Oleksandr Natalenko]

Move MADV_MERGEABLE part of ksm_madvise() into a dedicated helper since
it will be further used in do_anonymous_page().

This does not bring any functional changes.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
---
 include/linux/ksm.h |  2 ++
 mm/ksm.c            | 66 ++++++++++++++++++++++++++-------------------
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/include/linux/ksm.h b/include/linux/ksm.h
index e48b1e453ff5..bc13f228e2ed 100644
--- a/include/linux/ksm.h
+++ b/include/linux/ksm.h
@@ -21,6 +21,8 @@ struct mem_cgroup;
 #ifdef CONFIG_KSM
 int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 		unsigned long end, int advice, unsigned long *vm_flags);
+int ksm_enter(struct mm_struct *mm, struct vm_area_struct *vma,
+		unsigned long *vm_flags);
 int __ksm_enter(struct mm_struct *mm);
 void __ksm_exit(struct mm_struct *mm);
 
diff --git a/mm/ksm.c b/mm/ksm.c
index fc64874dc6f4..a6b0788a3a22 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2450,33 +2450,9 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 
 	switch (advice) {
 	case MADV_MERGEABLE:
-		/*
-		 * Be somewhat over-protective for now!
-		 */
-		if (*vm_flags & (VM_MERGEABLE | VM_SHARED  | VM_MAYSHARE   |
-				 VM_PFNMAP    | VM_IO      | VM_DONTEXPAND |
-				 VM_HUGETLB | VM_MIXEDMAP))
-			return 0;		/* just ignore the advice */
-
-		if (vma_is_dax(vma))
-			return 0;
-
-#ifdef VM_SAO
-		if (*vm_flags & VM_SAO)
-			return 0;
-#endif
-#ifdef VM_SPARC_ADI
-		if (*vm_flags & VM_SPARC_ADI)
-			return 0;
-#endif
-
-		if (!test_bit(MMF_VM_MERGEABLE, &mm->flags)) {
-			err = __ksm_enter(mm);
-			if (err)
-				return err;
-		}
-
-		*vm_flags |= VM_MERGEABLE;
+		err = ksm_enter(mm, vma, vm_flags);
+		if (err)
+			return err;
 		break;
 
 	case MADV_UNMERGEABLE:
@@ -2496,6 +2472,42 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 	return 0;
 }
 
+int ksm_enter(struct mm_struct *mm, struct vm_area_struct *vma,
+		unsigned long *vm_flags)
+{
+	int err;
+
+	/*
+	 * Be somewhat over-protective for now!
+	 */
+	if (*vm_flags & (VM_MERGEABLE | VM_SHARED  | VM_MAYSHARE   |
+			 VM_PFNMAP    | VM_IO      | VM_DONTEXPAND |
+			 VM_HUGETLB | VM_MIXEDMAP))
+		return 0;		/* just ignore the advice */
+
+	if (vma_is_dax(vma))
+		return 0;
+
+#ifdef VM_SAO
+	if (*vm_flags & VM_SAO)
+		return 0;
+#endif
+#ifdef VM_SPARC_ADI
+	if (*vm_flags & VM_SPARC_ADI)
+		return 0;
+#endif
+
+	if (!test_bit(MMF_VM_MERGEABLE, &mm->flags)) {
+		err = __ksm_enter(mm);
+		if (err)
+			return err;
+	}
+
+	*vm_flags |= VM_MERGEABLE;
+
+	return 0;
+}
+
 int __ksm_enter(struct mm_struct *mm)
 {
 	struct mm_slot *mm_slot;
-- 
2.21.0

[PATCH RFC 2/4] mm/ksm: introduce VM_UNMERGEABLE

[Oleksandr Natalenko]

Add separate vmaflag to allow applications to opt out of automatic VMAs
merging due to (possible) security concerns.

Since vmaflags are tight on free bits, this flag is available on 64-bit
architectures only. Thus, subsequently, KSM "always" mode will be
available for 64-bit architectures only as well.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
---
 fs/proc/task_mmu.c             |  3 +++
 include/linux/mm.h             |  6 ++++++
 include/trace/events/mmflags.h |  7 +++++++
 mm/ksm.c                       | 13 +++++++++++++
 4 files changed, 29 insertions(+)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 95ca1fe7283c..19cc246000e8 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -648,6 +648,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 		[ilog2(VM_MIXEDMAP)]	= "mm",
 		[ilog2(VM_HUGEPAGE)]	= "hg",
 		[ilog2(VM_NOHUGEPAGE)]	= "nh",
+#ifdef VM_UNMERGEABLE
+		[ilog2(VM_UNMERGEABLE)]	= "ug",
+#endif
 		[ilog2(VM_MERGEABLE)]	= "mg",
 		[ilog2(VM_UFFD_MISSING)]= "um",
 		[ilog2(VM_UFFD_WP)]	= "uw",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6b10c21630f5..114cdb882cdd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -252,11 +252,13 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_HIGH_ARCH_BIT_2	34	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_3	35	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_4	36	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_5	37	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_0	BIT(VM_HIGH_ARCH_BIT_0)
 #define VM_HIGH_ARCH_1	BIT(VM_HIGH_ARCH_BIT_1)
 #define VM_HIGH_ARCH_2	BIT(VM_HIGH_ARCH_BIT_2)
 #define VM_HIGH_ARCH_3	BIT(VM_HIGH_ARCH_BIT_3)
 #define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
+#define VM_HIGH_ARCH_5	BIT(VM_HIGH_ARCH_BIT_5)
 #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
@@ -272,6 +274,10 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 #endif /* CONFIG_ARCH_HAS_PKEYS */
 
+#ifdef VM_HIGH_ARCH_5
+#define VM_UNMERGEABLE	VM_HIGH_ARCH_5	/* Opt-out for KSM "always" mode */
+#endif /* VM_HIGH_ARCH_5 */
+
 #if defined(CONFIG_X86)
 # define VM_PAT		VM_ARCH_1	/* PAT reserves whole VMA at once (x86) */
 #elif defined(CONFIG_PPC)
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index a1675d43777e..717e0fd9d2ef 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -130,6 +130,12 @@ IF_HAVE_PG_IDLE(PG_idle,		"idle"		)
 #define IF_HAVE_VM_SOFTDIRTY(flag,name)
 #endif
 
+#ifdef VM_UNMERGEABLE
+#define IF_HAVE_VM_UNMERGEABLE(flag,name) {flag, name },
+#else
+#define IF_HAVE_VM_UNMERGEABLE(flag,name)
+#endif
+
 #define __def_vmaflag_names						\
 	{VM_READ,			"read"		},		\
 	{VM_WRITE,			"write"		},		\
@@ -161,6 +167,7 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY,	"softdirty"	)		\
 	{VM_MIXEDMAP,			"mixedmap"	},		\
 	{VM_HUGEPAGE,			"hugepage"	},		\
 	{VM_NOHUGEPAGE,			"nohugepage"	},		\
+IF_HAVE_VM_UNMERGEABLE(VM_UNMERGEABLE,	"unmergeable"	)		\
 	{VM_MERGEABLE,			"mergeable"	}		\
 
 #define show_vma_flags(flags)						\
diff --git a/mm/ksm.c b/mm/ksm.c
index a6b0788a3a22..0fb5f850087a 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -2450,12 +2450,18 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 
 	switch (advice) {
 	case MADV_MERGEABLE:
+#ifdef VM_UNMERGEABLE
+		*vm_flags &= ~VM_UNMERGEABLE;
+#endif
 		err = ksm_enter(mm, vma, vm_flags);
 		if (err)
 			return err;
 		break;
 
 	case MADV_UNMERGEABLE:
+#ifdef VM_UNMERGEABLE
+		*vm_flags |= VM_UNMERGEABLE;
+#endif
 		if (!(*vm_flags & VM_MERGEABLE))
 			return 0;		/* just ignore the advice */
 
@@ -2496,6 +2502,10 @@ int ksm_enter(struct mm_struct *mm, struct vm_area_struct *vma,
 	if (*vm_flags & VM_SPARC_ADI)
 		return 0;
 #endif
+#ifdef VM_UNMERGEABLE
+	if (*vm_flags & VM_UNMERGEABLE)
+		return 0;
+#endif
 
 	if (!test_bit(MMF_VM_MERGEABLE, &mm->flags)) {
 		err = __ksm_enter(mm);
@@ -3173,6 +3183,9 @@ static ssize_t full_scans_show(struct kobject *kobj,
 KSM_ATTR_RO(full_scans);
 
 static struct attribute *ksm_attrs[] = {
+#ifdef VM_UNMERGEABLE
+	&mode_attr.attr,
+#endif
 	&sleep_millisecs_attr.attr,
 	&pages_to_scan_attr.attr,
 	&run_attr.attr,
-- 
2.21.0

[PATCH RFC 3/4] mm/ksm: allow anonymous memory automerging

[Oleksandr Natalenko]

Introduce 2 KSM modes:

  * madvise, which is default and maintains old behaviour; and
  * always, in which new anonymous allocations are marked as eligible
    for merging.

The mode is controlled either via sysfs or via kernel cmdline for VMAs
to be marked as soon as possible during the boot process.

Previously introduced ksm_enter() helper is used to hook into
do_anonymous_page() and mark each eligible VMA as ready for merging.
This avoids introducing separate kthread to walk through the task/VMAs
list.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
---
 include/linux/ksm.h |  3 +++
 mm/ksm.c            | 65 +++++++++++++++++++++++++++++++++++++++++++++
 mm/memory.c         |  6 +++++
 3 files changed, 74 insertions(+)

diff --git a/include/linux/ksm.h b/include/linux/ksm.h
index bc13f228e2ed..3c076b35259c 100644
--- a/include/linux/ksm.h
+++ b/include/linux/ksm.h
@@ -21,6 +21,9 @@ struct mem_cgroup;
 #ifdef CONFIG_KSM
 int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 		unsigned long end, int advice, unsigned long *vm_flags);
+#ifdef VM_UNMERGEABLE
+bool ksm_mode_always(void);
+#endif
 int ksm_enter(struct mm_struct *mm, struct vm_area_struct *vma,
 		unsigned long *vm_flags);
 int __ksm_enter(struct mm_struct *mm);
diff --git a/mm/ksm.c b/mm/ksm.c
index 0fb5f850087a..6a2280b875cc 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -295,6 +295,12 @@ static int ksm_nr_node_ids = 1;
 static unsigned long ksm_run = KSM_RUN_STOP;
 static void wait_while_offlining(void);
 
+#ifdef VM_UNMERGEABLE
+#define KSM_MODE_MADVISE	0
+#define KSM_MODE_ALWAYS		1
+static unsigned long ksm_mode = KSM_MODE_MADVISE;
+#endif
+
 static DECLARE_WAIT_QUEUE_HEAD(ksm_thread_wait);
 static DECLARE_WAIT_QUEUE_HEAD(ksm_iter_wait);
 static DEFINE_MUTEX(ksm_thread_mutex);
@@ -2478,6 +2484,36 @@ int ksm_madvise(struct vm_area_struct *vma, unsigned long start,
 	return 0;
 }
 
+#ifdef VM_UNMERGEABLE
+bool ksm_mode_always(void)
+{
+	return ksm_mode == KSM_MODE_ALWAYS;
+}
+
+static int __init setup_ksm_mode(char *str)
+{
+	int ret = 0;
+
+	if (!str)
+		goto out;
+
+	if (!strcmp(str, "madvise")) {
+		ksm_mode = KSM_MODE_MADVISE;
+		ret = 1;
+	} else if (!strcmp(str, "always")) {
+		ksm_mode = KSM_MODE_ALWAYS;
+		ret = 1;
+	}
+
+out:
+	if (!ret)
+		pr_warn("ksm_mode= cannot parse, ignored\n");
+
+	return ret;
+}
+__setup("ksm_mode=", setup_ksm_mode);
+#endif
+
 int ksm_enter(struct mm_struct *mm, struct vm_area_struct *vma,
 		unsigned long *vm_flags)
 {
@@ -2881,6 +2917,35 @@ static void wait_while_offlining(void)
 	static struct kobj_attribute _name##_attr = \
 		__ATTR(_name, 0644, _name##_show, _name##_store)
 
+#ifdef VM_UNMERGEABLE
+static ssize_t mode_show(struct kobject *kobj, struct kobj_attribute *attr,
+	char *buf)
+{
+	switch (ksm_mode) {
+	case KSM_MODE_MADVISE:
+		return sprintf(buf, "always [madvise]\n");
+	case KSM_MODE_ALWAYS:
+		return sprintf(buf, "[always] madvise\n");
+	}
+
+	return sprintf(buf, "always [madvise]\n");
+}
+
+static ssize_t mode_store(struct kobject *kobj, struct kobj_attribute *attr,
+	const char *buf, size_t count)
+{
+	if (!memcmp("madvise", buf, min(sizeof("madvise")-1, count)))
+		ksm_mode = KSM_MODE_MADVISE;
+	else if (!memcmp("always", buf, min(sizeof("always")-1, count)))
+		ksm_mode = KSM_MODE_ALWAYS;
+	else
+		return -EINVAL;
+
+	return count;
+}
+KSM_ATTR(mode);
+#endif
+
 static ssize_t sleep_millisecs_show(struct kobject *kobj,
 				    struct kobj_attribute *attr, char *buf)
 {
diff --git a/mm/memory.c b/mm/memory.c
index ab650c21bccd..08f3f92de310 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2994,6 +2994,12 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
 	update_mmu_cache(vma, vmf->address, vmf->pte);
 unlock:
 	pte_unmap_unlock(vmf->pte, vmf->ptl);
+
+#if defined(CONFIG_KSM) && defined(VM_UNMERGEABLE)
+	if (ksm_mode_always())
+		ksm_enter(vma->vm_mm, vma, &vma->vm_flags);
+#endif
+
 	return ret;
 release:
 	mem_cgroup_cancel_charge(page, memcg, false);
-- 
2.21.0

[PATCH RFC 4/4] mm/ksm: add automerging documentation

[Oleksandr Natalenko]

Document KSM "always" mode kernel cmdline option as well as
corresponding sysfs knob.

Signed-off-by: Oleksandr Natalenko <oleksandr@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 Documentation/admin-guide/mm/ksm.rst            | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb644..510766a3fa05 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2008,6 +2008,13 @@
 			0: force disabled
 			1: force enabled
 
+	ksm_mode=
+			[KNL]
+			Format: [madvise|always]
+			Default: madvise
+			Can be used to control the default behavior of the system
+			with respect to merging anonymous memory.
+
 	kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs.
 			Default is 0 (don't ignore, but inject #GP)
 
diff --git a/Documentation/admin-guide/mm/ksm.rst b/Documentation/admin-guide/mm/ksm.rst
index 9303786632d1..9af730640da7 100644
--- a/Documentation/admin-guide/mm/ksm.rst
+++ b/Documentation/admin-guide/mm/ksm.rst
@@ -78,6 +78,13 @@ KSM daemon sysfs interface
 The KSM daemon is controlled by sysfs files in ``/sys/kernel/mm/ksm/``,
 readable by all but writable only by root:
 
+mode
+        * set madvise to deduplicate only madvised memory
+        * set always to allow deduplicating all the anonymous memory
+          (applies to newly allocated memory only)
+
+        Default: madvise (maintains old behaviour)
+
 pages_to_scan
         how many pages to scan before ksmd goes to sleep
         e.g. ``echo 100 > /sys/kernel/mm/ksm/pages_to_scan``.
-- 
2.21.0

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Kirill Tkhai]

Hi, Oleksandr,

On 10.05.2019 10:21, Oleksandr Natalenko wrote:
> By default, KSM works only on memory that is marked by madvise(). And the
> only way to get around that is to either:
> 
>   * use LD_PRELOAD; or
>   * patch the kernel with something like UKSM or PKSM.
>
> Instead, lets implement a so-called "always" mode, which allows marking
> VMAs as mergeable on do_anonymous_page() call automatically.
>
> The submission introduces a new sysctl knob as well as kernel cmdline option
> to control which mode to use. The default mode is to maintain old
> (madvise-based) behaviour.
>
> Due to security concerns, this submission also introduces VM_UNMERGEABLE
> vmaflag for apps to explicitly opt out of automerging. Because of adding
> a new vmaflag, the whole work is available for 64-bit architectures only.
>> This patchset is based on earlier Timofey's submission [1], but it doesn't
> use dedicated kthread to walk through the list of tasks/VMAs.
> 
> For my laptop it saves up to 300 MiB of RAM for usual workflow (browser,
> terminal, player, chats etc). Timofey's submission also mentions
> containerised workload that benefits from automerging too.

This all approach looks complicated for me, and I'm not sure the shown profit
for desktop is big enough to introduce contradictory vma flags, boot option
and advance page fault handler. Also, 32/64bit defines do not look good for
me. I had tried something like this on my laptop some time ago, and
the result was bad even in absolute (not in memory percentage) meaning.
Isn't LD_PRELOAD trick enough to desktop? Your workload is same all the time,
so you may statically insert correct preload to /etc/profile and replace
your mmap forever.

Speaking about containers, something like this may have a sense, I think.
The probability of that several containers have the same pages are higher,
than that desktop applications have the same pages; also LD_PRELOAD for
containers is not applicable. 

But 1)this could be made for trusted containers only (are there similar
issues with KSM like with hardware side-channel attacks?!); 2) the most
shared data for containers in my experience is file cache, which is not
supported by KSM.

There are good results by the link [1], but it's difficult to analyze
them without knowledge about what happens inside them there.

Some of tests have "VM" prefix. What the reason the hypervisor don't mark
their VMAs as mergeable? Can't this be fixed in hypervisor? What is the
generic reason that VMAs are not marked in all the tests?

In case of there is a fundamental problem of calling madvise, can't we
just implement an easier workaround like a new write-only file:

#echo $task > /sys/kernel/mm/ksm/force_madvise

which will mark all anon VMAs as mergeable for a passed task's mm?

A small userspace daemon may write mergeable tasks there from time to time.

Then we won't need to introduce additional vm flags and to change
anon pagefault handler, and the changes will be small and only
related to mm/ksm.c, and good enough for both 32 and 64 bit machines.

Thanks,
Kirill

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

Hi.

On Mon, May 13, 2019 at 01:38:43PM +0300, Kirill Tkhai wrote:
> On 10.05.2019 10:21, Oleksandr Natalenko wrote:
> > By default, KSM works only on memory that is marked by madvise(). And the
> > only way to get around that is to either:
> > 
> >   * use LD_PRELOAD; or
> >   * patch the kernel with something like UKSM or PKSM.
> >
> > Instead, lets implement a so-called "always" mode, which allows marking
> > VMAs as mergeable on do_anonymous_page() call automatically.
> >
> > The submission introduces a new sysctl knob as well as kernel cmdline option
> > to control which mode to use. The default mode is to maintain old
> > (madvise-based) behaviour.
> >
> > Due to security concerns, this submission also introduces VM_UNMERGEABLE
> > vmaflag for apps to explicitly opt out of automerging. Because of adding
> > a new vmaflag, the whole work is available for 64-bit architectures only.
> >> This patchset is based on earlier Timofey's submission [1], but it doesn't
> > use dedicated kthread to walk through the list of tasks/VMAs.
> > 
> > For my laptop it saves up to 300 MiB of RAM for usual workflow (browser,
> > terminal, player, chats etc). Timofey's submission also mentions
> > containerised workload that benefits from automerging too.
> 
> This all approach looks complicated for me, and I'm not sure the shown profit
> for desktop is big enough to introduce contradictory vma flags, boot option
> and advance page fault handler. Also, 32/64bit defines do not look good for
> me. I had tried something like this on my laptop some time ago, and
> the result was bad even in absolute (not in memory percentage) meaning.
> Isn't LD_PRELOAD trick enough to desktop? Your workload is same all the time,
> so you may statically insert correct preload to /etc/profile and replace
> your mmap forever.
>
> Speaking about containers, something like this may have a sense, I think.
> The probability of that several containers have the same pages are higher,
> than that desktop applications have the same pages; also LD_PRELOAD for
> containers is not applicable. 

Yes, I get your point. But the intention is to avoid another hacky trick
(LD_PRELOAD), thus *something* should *preferably* be done on the
kernel level instead.

> But 1)this could be made for trusted containers only (are there similar
> issues with KSM like with hardware side-channel attacks?!);

Regarding side-channel attacks, yes, I think so. Were those openssl guys
who complained about it?..

> 2) the most
> shared data for containers in my experience is file cache, which is not
> supported by KSM.
> 
> There are good results by the link [1], but it's difficult to analyze
> them without knowledge about what happens inside them there.
> 
> Some of tests have "VM" prefix. What the reason the hypervisor don't mark
> their VMAs as mergeable? Can't this be fixed in hypervisor? What is the
> generic reason that VMAs are not marked in all the tests?

Timofey, could you please address this?

Also, just for the sake of another piece of stats here:

$ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
526

> In case of there is a fundamental problem of calling madvise, can't we
> just implement an easier workaround like a new write-only file:
> 
> #echo $task > /sys/kernel/mm/ksm/force_madvise
> 
> which will mark all anon VMAs as mergeable for a passed task's mm?
> 
> A small userspace daemon may write mergeable tasks there from time to time.
> 
> Then we won't need to introduce additional vm flags and to change
> anon pagefault handler, and the changes will be small and only
> related to mm/ksm.c, and good enough for both 32 and 64 bit machines.

Yup, looks appealing. Two concerns, though:

1) we are falling back to scanning through the list of tasks (I guess
this is what we wanted to avoid, although this time it happens in the
userspace);

2) what kinds of opt-out we should maintain? Like, what if force_madvise
is called, but the task doesn't want some VMAs to be merged? This will
required new flag anyway, it seems. And should there be another
write-only file to unmerge everything forcibly for specific task?

Thanks.

P.S. Cc'ing Pavel properly this time.

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Senior Software Maintenance Engineer

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Timofey Titovets]

пн, 13 мая 2019 г. в 14:33, Oleksandr Natalenko <oleksandr@redhat.com>:
>
> Hi.
>
> On Mon, May 13, 2019 at 01:38:43PM +0300, Kirill Tkhai wrote:
> > On 10.05.2019 10:21, Oleksandr Natalenko wrote:
> > > By default, KSM works only on memory that is marked by madvise(). And the
> > > only way to get around that is to either:
> > >
> > >   * use LD_PRELOAD; or
> > >   * patch the kernel with something like UKSM or PKSM.
> > >
> > > Instead, lets implement a so-called "always" mode, which allows marking
> > > VMAs as mergeable on do_anonymous_page() call automatically.
> > >
> > > The submission introduces a new sysctl knob as well as kernel cmdline option
> > > to control which mode to use. The default mode is to maintain old
> > > (madvise-based) behaviour.
> > >
> > > Due to security concerns, this submission also introduces VM_UNMERGEABLE
> > > vmaflag for apps to explicitly opt out of automerging. Because of adding
> > > a new vmaflag, the whole work is available for 64-bit architectures only.
> > >> This patchset is based on earlier Timofey's submission [1], but it doesn't
> > > use dedicated kthread to walk through the list of tasks/VMAs.
> > >
> > > For my laptop it saves up to 300 MiB of RAM for usual workflow (browser,
> > > terminal, player, chats etc). Timofey's submission also mentions
> > > containerised workload that benefits from automerging too.
> >
> > This all approach looks complicated for me, and I'm not sure the shown profit
> > for desktop is big enough to introduce contradictory vma flags, boot option
> > and advance page fault handler. Also, 32/64bit defines do not look good for
> > me. I had tried something like this on my laptop some time ago, and
> > the result was bad even in absolute (not in memory percentage) meaning.
> > Isn't LD_PRELOAD trick enough to desktop? Your workload is same all the time,
> > so you may statically insert correct preload to /etc/profile and replace
> > your mmap forever.
> >
> > Speaking about containers, something like this may have a sense, I think.
> > The probability of that several containers have the same pages are higher,
> > than that desktop applications have the same pages; also LD_PRELOAD for
> > containers is not applicable.
>
> Yes, I get your point. But the intention is to avoid another hacky trick
> (LD_PRELOAD), thus *something* should *preferably* be done on the
> kernel level instead.
>
> > But 1)this could be made for trusted containers only (are there similar
> > issues with KSM like with hardware side-channel attacks?!);
>
> Regarding side-channel attacks, yes, I think so. Were those openssl guys
> who complained about it?..
>
> > 2) the most
> > shared data for containers in my experience is file cache, which is not
> > supported by KSM.
> >
> > There are good results by the link [1], but it's difficult to analyze
> > them without knowledge about what happens inside them there.
> >
> > Some of tests have "VM" prefix. What the reason the hypervisor don't mark
> > their VMAs as mergeable? Can't this be fixed in hypervisor? What is the
> > generic reason that VMAs are not marked in all the tests?
>
> Timofey, could you please address this?

That's just a describe of machine,
only to show difference in deduplication for application in small VM
and real big server
i.e. KSM enabled in VM for containers, not for hypervisor.

> Also, just for the sake of another piece of stats here:
>
> $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> 526

IIRC, for calculate saving you must use (pages_shared - pages_sharing)

> > In case of there is a fundamental problem of calling madvise, can't we
> > just implement an easier workaround like a new write-only file:
> >
> > #echo $task > /sys/kernel/mm/ksm/force_madvise
> >
> > which will mark all anon VMAs as mergeable for a passed task's mm?
> >
> > A small userspace daemon may write mergeable tasks there from time to time.
> >
> > Then we won't need to introduce additional vm flags and to change
> > anon pagefault handler, and the changes will be small and only
> > related to mm/ksm.c, and good enough for both 32 and 64 bit machines.
>
> Yup, looks appealing. Two concerns, though:
>
> 1) we are falling back to scanning through the list of tasks (I guess
> this is what we wanted to avoid, although this time it happens in the
> userspace);
>
> 2) what kinds of opt-out we should maintain? Like, what if force_madvise
> is called, but the task doesn't want some VMAs to be merged? This will
> required new flag anyway, it seems. And should there be another
> write-only file to unmerge everything forcibly for specific task?
>
> Thanks.
>
> P.S. Cc'ing Pavel properly this time.
>
> --
>   Best regards,
>     Oleksandr Natalenko (post-factum)
>     Senior Software Maintenance Engineer



-- 
Have a nice day,
Timofey.

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

On Mon, May 13, 2019 at 02:48:29PM +0300, Timofey Titovets wrote:
> > Also, just for the sake of another piece of stats here:
> >
> > $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> > 526
> 
> IIRC, for calculate saving you must use (pages_shared - pages_sharing)

Based on Documentation/ABI/testing/sysfs-kernel-mm-ksm:

	pages_shared: how many shared pages are being used.

	pages_sharing: how many more sites are sharing them i.e. how
	much saved.

and unless I'm missing something, this must be already accounted:

[~]$ echo "$(cat /sys/kernel/mm/ksm/pages_shared) * 4 / 1024" | bc
69

[~]$ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
563

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Senior Software Maintenance Engineer

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

On Mon, May 13, 2019 at 02:01:17PM +0200, Oleksandr Natalenko wrote:
> On Mon, May 13, 2019 at 02:48:29PM +0300, Timofey Titovets wrote:
> > > Also, just for the sake of another piece of stats here:
> > >
> > > $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> > > 526
> > 
> > IIRC, for calculate saving you must use (pages_shared - pages_sharing)
> 
> Based on Documentation/ABI/testing/sysfs-kernel-mm-ksm:
> 
> 	pages_shared: how many shared pages are being used.
> 
> 	pages_sharing: how many more sites are sharing them i.e. how
> 	much saved.
> 
> and unless I'm missing something, this must be already accounted:
> 
> [~]$ echo "$(cat /sys/kernel/mm/ksm/pages_shared) * 4 / 1024" | bc
> 69
> 
> [~]$ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> 563

Yup. To expand on this,

 246 /* The number of nodes in the stable tree */
 247 static unsigned long ksm_pages_shared;
 248 
 249 /* The number of page slots additionally sharing those nodes */
 250 static unsigned long ksm_pages_sharing;

2037     if (rmap_item->hlist.next)
2038         ksm_pages_sharing++;
2039     else
2040         ksm_pages_shared++;

IOW, first item is accounter in "shared", the rest will go to "sharing".

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Senior Software Maintenance Engineer

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Kirill Tkhai]

On 13.05.2019 14:33, Oleksandr Natalenko wrote:
> Hi.
> 
> On Mon, May 13, 2019 at 01:38:43PM +0300, Kirill Tkhai wrote:
>> On 10.05.2019 10:21, Oleksandr Natalenko wrote:
>>> By default, KSM works only on memory that is marked by madvise(). And the
>>> only way to get around that is to either:
>>>
>>>   * use LD_PRELOAD; or
>>>   * patch the kernel with something like UKSM or PKSM.
>>>
>>> Instead, lets implement a so-called "always" mode, which allows marking
>>> VMAs as mergeable on do_anonymous_page() call automatically.
>>>
>>> The submission introduces a new sysctl knob as well as kernel cmdline option
>>> to control which mode to use. The default mode is to maintain old
>>> (madvise-based) behaviour.
>>>
>>> Due to security concerns, this submission also introduces VM_UNMERGEABLE
>>> vmaflag for apps to explicitly opt out of automerging. Because of adding
>>> a new vmaflag, the whole work is available for 64-bit architectures only.
>>>> This patchset is based on earlier Timofey's submission [1], but it doesn't
>>> use dedicated kthread to walk through the list of tasks/VMAs.
>>>
>>> For my laptop it saves up to 300 MiB of RAM for usual workflow (browser,
>>> terminal, player, chats etc). Timofey's submission also mentions
>>> containerised workload that benefits from automerging too.
>>
>> This all approach looks complicated for me, and I'm not sure the shown profit
>> for desktop is big enough to introduce contradictory vma flags, boot option
>> and advance page fault handler. Also, 32/64bit defines do not look good for
>> me. I had tried something like this on my laptop some time ago, and
>> the result was bad even in absolute (not in memory percentage) meaning.
>> Isn't LD_PRELOAD trick enough to desktop? Your workload is same all the time,
>> so you may statically insert correct preload to /etc/profile and replace
>> your mmap forever.
>>
>> Speaking about containers, something like this may have a sense, I think.
>> The probability of that several containers have the same pages are higher,
>> than that desktop applications have the same pages; also LD_PRELOAD for
>> containers is not applicable. 
> 
> Yes, I get your point. But the intention is to avoid another hacky trick
> (LD_PRELOAD), thus *something* should *preferably* be done on the
> kernel level instead.

I don't think so. Does userspace hack introduce some overhead? It does not
look so. Why should we think about mergeable VMAs in page fault handler?!
This is the last thing we want to think in page fault handler.

Also, there is difficult synchronization in page fault handlers, and it's
easy to make a mistake. So, there is a mistake in [3/4], and you call
ksm_enter() with mmap_sem read locked, while normal way is to call it
with write lock (see madvise_need_mmap_write()).

So, let's don't touch this path. Small optimization for unlikely case will
introduce problems in optimization for likely case in the future.

>> But 1)this could be made for trusted containers only (are there similar
>> issues with KSM like with hardware side-channel attacks?!);
> 
> Regarding side-channel attacks, yes, I think so. Were those openssl guys
> who complained about it?..
> 
>> 2) the most
>> shared data for containers in my experience is file cache, which is not
>> supported by KSM.
>>
>> There are good results by the link [1], but it's difficult to analyze
>> them without knowledge about what happens inside them there.
>>
>> Some of tests have "VM" prefix. What the reason the hypervisor don't mark
>> their VMAs as mergeable? Can't this be fixed in hypervisor? What is the
>> generic reason that VMAs are not marked in all the tests?
> 
> Timofey, could you please address this?
> 
> Also, just for the sake of another piece of stats here:
> 
> $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> 526

This all requires attentive analysis. The number looks pretty big for me.
What are the pages you get merged there? This may be just zero pages,
you have identical.

E.g., your browser want to work fast. It introduces smart schemes,
and preallocates many pages in background (mmap + write 1 byte to a page),
so in further it save some time (no page fault + alloc), when page is
really needed. But your change merges these pages and kills this
optimization. Sounds not good, does this?

I think, we should not think we know and predict better than application
writers, what they want from kernel. Let's people decide themselves
in dependence of their workload. The only exception is some buggy
or old applications, which impossible to change, so force madvise
workaround may help. But only in case there are really such applications...

I'd researched what pages you have duplicated in these 526 MB. Maybe
you find, no action is required or a report to userspace application
to use madvise is needed.

>> In case of there is a fundamental problem of calling madvise, can't we
>> just implement an easier workaround like a new write-only file:
>>
>> #echo $task > /sys/kernel/mm/ksm/force_madvise
>>
>> which will mark all anon VMAs as mergeable for a passed task's mm?
>>
>> A small userspace daemon may write mergeable tasks there from time to time.
>>
>> Then we won't need to introduce additional vm flags and to change
>> anon pagefault handler, and the changes will be small and only
>> related to mm/ksm.c, and good enough for both 32 and 64 bit machines.
> 
> Yup, looks appealing. Two concerns, though:
> 
> 1) we are falling back to scanning through the list of tasks (I guess
> this is what we wanted to avoid, although this time it happens in the
> userspace);

IMO, this should be made only for tasks, which are known to be buggy
(which can't call madvise). Yes, scanning will be required.

> 2) what kinds of opt-out we should maintain? Like, what if force_madvise
> is called, but the task doesn't want some VMAs to be merged? This will
> required new flag anyway, it seems. And should there be another
> write-only file to unmerge everything forcibly for specific task?

For example,

Merge:
#echo $task > /sys/kernel/mm/ksm/force_madvise

Unmerge:
#echo -$task > /sys/kernel/mm/ksm/force_madvise

In case of task don't want to merge some VMA, we just should skip it at all.

But firstly we probably should check, that we really need this, and why
existing applications don't call madvise directly. Now we just don't know,
what happens.

Kirill

P.S. This all above is my opinion. Let's wait, what other people think.

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

Hi.

On Mon, May 13, 2019 at 03:37:56PM +0300, Kirill Tkhai wrote:
> > Yes, I get your point. But the intention is to avoid another hacky trick
> > (LD_PRELOAD), thus *something* should *preferably* be done on the
> > kernel level instead.
> 
> I don't think so. Does userspace hack introduce some overhead? It does not
> look so. Why should we think about mergeable VMAs in page fault handler?!
> This is the last thing we want to think in page fault handler.
> 
> Also, there is difficult synchronization in page fault handlers, and it's
> easy to make a mistake. So, there is a mistake in [3/4], and you call
> ksm_enter() with mmap_sem read locked, while normal way is to call it
> with write lock (see madvise_need_mmap_write()).
> 
> So, let's don't touch this path. Small optimization for unlikely case will
> introduce problems in optimization for likely case in the future.

Yup, you're right, I've missed the fact that write lock is needed there.
Re-vamping locking there is not my intention, so lets find another
solution.

> > Also, just for the sake of another piece of stats here:
> > 
> > $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
> > 526
> 
> This all requires attentive analysis. The number looks pretty big for me.
> What are the pages you get merged there? This may be just zero pages,
> you have identical.
> 
> E.g., your browser want to work fast. It introduces smart schemes,
> and preallocates many pages in background (mmap + write 1 byte to a page),
> so in further it save some time (no page fault + alloc), when page is
> really needed. But your change merges these pages and kills this
> optimization. Sounds not good, does this?
> 
> I think, we should not think we know and predict better than application
> writers, what they want from kernel. Let's people decide themselves
> in dependence of their workload. The only exception is some buggy
> or old applications, which impossible to change, so force madvise
> workaround may help. But only in case there are really such applications...
> 
> I'd researched what pages you have duplicated in these 526 MB. Maybe
> you find, no action is required or a report to userspace application
> to use madvise is needed.

OK, I agree, this is a good argument to move decision to userspace.

> > 2) what kinds of opt-out we should maintain? Like, what if force_madvise
> > is called, but the task doesn't want some VMAs to be merged? This will
> > required new flag anyway, it seems. And should there be another
> > write-only file to unmerge everything forcibly for specific task?
> 
> For example,
> 
> Merge:
> #echo $task > /sys/kernel/mm/ksm/force_madvise

Immediate question: what should be actually done on this? I see 2
options:

1) mark all VMAs as mergeable + set some flag for mmap() to mark all
further allocations as mergeable as well;
2) just mark all the VMAs as mergeable; userspace can call this
periodically to mark new VMAs.

My prediction is that 2) is less destructive, and the decision is
preserved predominantly to userspace, thus it would be a desired option.

> Unmerge:
> #echo -$task > /sys/kernel/mm/ksm/force_madvise

Okay.

> In case of task don't want to merge some VMA, we just should skip it at all.

This way we lose some flexibility, IMO, but I get you point.

Thanks.

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Senior Software Maintenance Engineer

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Kirill Tkhai]

On 14.05.2019 09:30, Oleksandr Natalenko wrote:
> Hi.
> 
> On Mon, May 13, 2019 at 03:37:56PM +0300, Kirill Tkhai wrote:
>>> Yes, I get your point. But the intention is to avoid another hacky trick
>>> (LD_PRELOAD), thus *something* should *preferably* be done on the
>>> kernel level instead.
>>
>> I don't think so. Does userspace hack introduce some overhead? It does not
>> look so. Why should we think about mergeable VMAs in page fault handler?!
>> This is the last thing we want to think in page fault handler.
>>
>> Also, there is difficult synchronization in page fault handlers, and it's
>> easy to make a mistake. So, there is a mistake in [3/4], and you call
>> ksm_enter() with mmap_sem read locked, while normal way is to call it
>> with write lock (see madvise_need_mmap_write()).
>>
>> So, let's don't touch this path. Small optimization for unlikely case will
>> introduce problems in optimization for likely case in the future.
> 
> Yup, you're right, I've missed the fact that write lock is needed there.
> Re-vamping locking there is not my intention, so lets find another
> solution.
> 
>>> Also, just for the sake of another piece of stats here:
>>>
>>> $ echo "$(cat /sys/kernel/mm/ksm/pages_sharing) * 4 / 1024" | bc
>>> 526
>>
>> This all requires attentive analysis. The number looks pretty big for me.
>> What are the pages you get merged there? This may be just zero pages,
>> you have identical.
>>
>> E.g., your browser want to work fast. It introduces smart schemes,
>> and preallocates many pages in background (mmap + write 1 byte to a page),
>> so in further it save some time (no page fault + alloc), when page is
>> really needed. But your change merges these pages and kills this
>> optimization. Sounds not good, does this?
>>
>> I think, we should not think we know and predict better than application
>> writers, what they want from kernel. Let's people decide themselves
>> in dependence of their workload. The only exception is some buggy
>> or old applications, which impossible to change, so force madvise
>> workaround may help. But only in case there are really such applications...
>>
>> I'd researched what pages you have duplicated in these 526 MB. Maybe
>> you find, no action is required or a report to userspace application
>> to use madvise is needed.
> 
> OK, I agree, this is a good argument to move decision to userspace.
> 
>>> 2) what kinds of opt-out we should maintain? Like, what if force_madvise
>>> is called, but the task doesn't want some VMAs to be merged? This will
>>> required new flag anyway, it seems. And should there be another
>>> write-only file to unmerge everything forcibly for specific task?
>>
>> For example,
>>
>> Merge:
>> #echo $task > /sys/kernel/mm/ksm/force_madvise
> 
> Immediate question: what should be actually done on this? I see 2
> options:
> 
> 1) mark all VMAs as mergeable + set some flag for mmap() to mark all
> further allocations as mergeable as well;
> 2) just mark all the VMAs as mergeable; userspace can call this
> periodically to mark new VMAs.
> 
> My prediction is that 2) is less destructive, and the decision is
> preserved predominantly to userspace, thus it would be a desired option.

Let's see, how we use KSM now. It's good for virtual machines: people
install the same distribution in several VMs, and they have the same
packages and the same files. When you read a file inside VM, its pages
are file cache for the VM, but they are anonymous pages for host kernel.

Hypervisor marks VM memory as mergeable, and host KSM merges the same
anonymous pages together. Many of file cache inside VM is constant
content, so we have good KSM compression on such the file pages.
The result we have is explainable and expected.

But we don't know anything about pages, you have merged on your laptop.
We can't make any assumptions before analysis of applications, which
produce such the pages. Let's check what happens before we try to implement
some specific design (if we really need something to implement).

The rest is just technical details. We may implement everything we need
on top of this (even implement a polling of /proc/[pid]/maps and write
a task and address of vma to force_madvise or similar file).

Kirill

Re: [PATCH RFC 0/4] mm/ksm: add option to automerge VMAs

[Oleksandr Natalenko]

Hi.

On Tue, May 14, 2019 at 12:12:16PM +0300, Kirill Tkhai wrote:
> > Immediate question: what should be actually done on this? I see 2
> > options:
> > 
> > 1) mark all VMAs as mergeable + set some flag for mmap() to mark all
> > further allocations as mergeable as well;
> > 2) just mark all the VMAs as mergeable; userspace can call this
> > periodically to mark new VMAs.
> > 
> > My prediction is that 2) is less destructive, and the decision is
> > preserved predominantly to userspace, thus it would be a desired option.
> 
> Let's see, how we use KSM now. It's good for virtual machines: people
> install the same distribution in several VMs, and they have the same
> packages and the same files. When you read a file inside VM, its pages
> are file cache for the VM, but they are anonymous pages for host kernel.
> 
> Hypervisor marks VM memory as mergeable, and host KSM merges the same
> anonymous pages together. Many of file cache inside VM is constant
> content, so we have good KSM compression on such the file pages.
> The result we have is explainable and expected.

Yup, correct.

> But we don't know anything about pages, you have merged on your laptop.
> We can't make any assumptions before analysis of applications, which
> produce such the pages. Let's check what happens before we try to implement
> some specific design (if we really need something to implement).
> 
> The rest is just technical details. We may implement everything we need
> on top of this (even implement a polling of /proc/[pid]/maps and write
> a task and address of vma to force_madvise or similar file).

I'm not sure that reviewing all the applications falls under the scope
of this and/or similar submission. Personally I do not feel comfortable
reviewing Firefox code, for example.

But I do run 2 instances of FF, one for work stuff, one for personal stuff,
so merging its memory would be definitely beneficial for me. I believe I'm
not the only one doing this, and things are not limited to Firefox only, of
course.

Please consider checking a v2 submission I've just posted. It implements
your suggestion on "force_madvise" knob, and I find your feedback very
relevant and useful.

Thanks.

-- 
  Best regards,
    Oleksandr Natalenko (post-factum)
    Senior Software Maintenance Engineer