[PATCH 0/2] mm/mempolicy: shared policy limits for tmpfs/shmem/guest_memfd

[PATCH 0/2] mm/mempolicy: shared policy limits for tmpfs/shmem/guest_memfd

[David Hildenbrand (Arm)]

For some reason we thought it would be a good idea to let users create
as many shared policy ranges for shmem/tmpfs as they want. And
guest_memfd inherited that handling.

Fortunately, at least for guest_memfd we know that we shouldn't really
have more than one shared policy right now in practice.

While we now at least charge these allocations to the memcg, users can
still allocate possibly many, and cause trouble, for example, by having
long list walks under spinlocks.

Instead of trying to improve things, let's just stop the user from doing
stupid things.

Essentially: limit the number of shared policy ranges per inode to 1024
as default. Let the admin increase that limit. 1024 is likely still
plenty: who maps a shmem/tmpfs/guest_memfd file and configures that many
mempolicies on it through separate VMAs?

Maybe we should set the limit even lower ... but 1024 feels at least like a
start where no user case should be impacted.

Patch #2 can be considered a fix, but I am not tagging it like that.
It's been that way forever ... should we? The impact is primarily just
wasting kernel memory and doing a lot of expensive operation under
locks.

Patch #1 prepares for that by merging adjacent policies if they are equal,
to reduce the number of policies further if some user space actually does
something questionable on sub-ranges.

This is not super urgent but something that should be addressed.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
David Hildenbrand (Arm) (2):
      mm/mempolicy: merge adjacent shared policy ranges
      mm/mempolicy: limit shared policy ranges per inode

 Documentation/admin-guide/sysctl/vm.rst | 18 ++++++++
 include/linux/mempolicy.h               |  1 +
 mm/mempolicy.c                          | 74 +++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)

---

base-commit: c6e0cad6c9b4180bb229db06e5b062c5ce0723bd

change-id: 20261113-mempolicy_limit-6f043870d04b

--

Cheers,

David

[PATCH 1/2] mm/mempolicy: merge adjacent shared policy ranges

[David Hildenbrand (Arm)]

Shared policies can retain separate tree nodes for adjacent ranges with
identical policies. This can create unnecessary policy ranges especially
when individual VMAs cover individual file ranges, and user space sets
the policy on these VMAs independently.

Extend the existing replacement handling to merge ranges with equal policy
at either boundary. The existing replacement loop will then remove the old
nodes and installs a single node for the combined range. This also avoids
splitting an existing node when setting the same policy on a subrange.

Extending either boundary rules out splitting an existing node, so the
allocation retry can only occur with the originally requested range.

This change will make it easier to cap the number of shared policy
ranges per inode.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 mm/mempolicy.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 06b90903e8794..183bc3bd077cd 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -3208,6 +3208,23 @@ static int shared_policy_replace(struct shared_policy *sp, pgoff_t start,
 
 restart:
 	write_lock(&sp->lock);
+	if (new) {
+		/* Try to merge ranges with same policies. */
+		if (start) {
+			n = sp_lookup(sp, start - 1, start);
+			if (n && mpol_equal(n->policy, new->policy)) {
+				new->start = n->start;
+				start = new->start;
+			}
+		}
+		if (end != ULONG_MAX) {
+			n = sp_lookup(sp, end, end + 1);
+			if (n && mpol_equal(n->policy, new->policy)) {
+				new->end = n->end;
+				end = new->end;
+			}
+		}
+	}
 	n = sp_lookup(sp, start, end);
 	/* Take care of old policies in the same range. */
 	while (n && n->start < end) {

-- 
2.43.0

[PATCH 2/2] mm/mempolicy: limit shared policy ranges per inode

[David Hildenbrand (Arm)]

Shared NUMA policy ranges can accumulate on an inode after its mappings
are removed. Currently, these nodes are effectively unbounded (even though
the allcoations are charged).

An unbounded tree can cause long rwlock hold times when replacing or
freeing policies. Instead of trying to fix that, it sounds more like
having many nodes is actually only a scenario for processes trying to
cause trouble. Most use cases will probably only use a handful of them,
not millions.

Add vm.max_shared_policy_count (inspired by vm.max_map_count), defaulting
to 1024 ranges per inode. Reject updates with -ENOMEM if they would
increase the count beyond the limit, taking merging, splitting etc.
into account.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 Documentation/admin-guide/sysctl/vm.rst | 18 +++++++++++
 include/linux/mempolicy.h               |  1 +
 mm/mempolicy.c                          | 57 +++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
index 3cb0138e850c1..8ef63d933821b 100644
--- a/Documentation/admin-guide/sysctl/vm.rst
+++ b/Documentation/admin-guide/sysctl/vm.rst
@@ -41,6 +41,7 @@ Currently, these files are in /proc/sys/vm:
 - legacy_va_layout
 - lowmem_reserve_ratio
 - max_map_count
+- max_shared_policy_count
 - mem_profiling         (only if CONFIG_MEM_ALLOC_PROFILING=y)
 - memory_failure_early_kill
 - memory_failure_recovery
@@ -473,6 +474,23 @@ e.g., up to one or two maps per allocation.
 The default value is 65530.
 
 
+max_shared_policy_count
+=======================
+
+The maximum number of shared NUMA policy ranges stored on a single inode.
+This applies to tmpfs files, shared anonymous mappings, shmem-backed memfds
+and System V shared memory segments, and guest_memfds. The policies are
+shared by all mappings of the same inode and remain after all mappings are
+removed.
+
+An update that would increase the number of ranges beyond this limit fails
+with -ENOMEM; this includes splitting of existing ranges. Lowering the
+limit will not remove existing policies. Updates that preserve or reduce
+the count remain allowed even if it exceeds the limit.
+
+The default value is 1024. The minimum value is 1.
+
+
 mem_profiling
 ==============
 
diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
index cd8f944209ca4..828e7de6dee73 100644
--- a/include/linux/mempolicy.h
+++ b/include/linux/mempolicy.h
@@ -113,6 +113,7 @@ static inline bool mpol_equal(struct mempolicy *a, struct mempolicy *b)
 struct shared_policy {
 	struct rb_root root;
 	rwlock_t lock;
+	unsigned int nr_nodes;
 };
 struct sp_node {
 	struct rb_node nd;
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 183bc3bd077cd..a54c6423faeaa 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -106,6 +106,7 @@
 #include <linux/rmap.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
+#include <linux/sysctl.h>
 #include <linux/ctype.h>
 #include <linux/mm_inline.h>
 #include <linux/mmu_notifier.h>
@@ -130,6 +131,28 @@
 static struct kmem_cache *policy_cache;
 static struct kmem_cache *sn_cache;
 
+static int sysctl_max_shared_policy_count __read_mostly = 1024;
+
+#ifdef CONFIG_SYSCTL
+static const struct ctl_table mempolicy_sysctls[] = {
+	{
+		.procname	= "max_shared_policy_count",
+		.data		= &sysctl_max_shared_policy_count,
+		.maxlen		= sizeof(sysctl_max_shared_policy_count),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ONE,
+	},
+};
+
+static int __init mempolicy_sysctl_init(void)
+{
+	register_sysctl_init("vm", mempolicy_sysctls);
+	return 0;
+}
+subsys_initcall(mempolicy_sysctl_init);
+#endif
+
 /* Highest zone. An specific allocation for a zone below that is not
    policied. */
 enum zone_type policy_zone = 0;
@@ -3006,6 +3029,7 @@ static void sp_insert(struct shared_policy *sp, struct sp_node *new)
 	}
 	rb_link_node(&new->nd, parent, p);
 	rb_insert_color(&new->nd, &sp->root);
+	sp->nr_nodes++;
 }
 
 /* Find shared policy intersecting idx */
@@ -3165,6 +3189,7 @@ void mpol_put_task_policy(struct task_struct *task)
 static void sp_delete(struct shared_policy *sp, struct sp_node *n)
 {
 	rb_erase(&n->nd, &sp->root);
+	sp->nr_nodes--;
 	sp_free(n);
 }
 
@@ -3197,6 +3222,30 @@ static struct sp_node *sp_alloc(unsigned long start, unsigned long end,
 	return n;
 }
 
+static int sp_check_node_limit(struct shared_policy *sp, struct sp_node *n,
+			       pgoff_t start, pgoff_t end, bool new)
+{
+	unsigned long new_nodes = new ? 1 : 0;
+
+	if (n && n->start < start) {
+		/*
+		 * Splitting adds one node for the old policy. Otherwise,
+		 * check the successor.
+		 */
+		if (n->end > end)
+			new_nodes++;
+		else
+			n = rb_entry_safe(rb_next(&n->nd), struct sp_node, nd);
+	}
+	/* Without a new node, or with a whole node removed, the count cannot grow. */
+	if (!new_nodes || (n && n->end <= end))
+		return 0;
+
+	if (sp->nr_nodes + new_nodes > READ_ONCE(sysctl_max_shared_policy_count))
+		return -ENOMEM;
+	return 0;
+}
+
 /* Replace a policy range. */
 static int shared_policy_replace(struct shared_policy *sp, pgoff_t start,
 				 pgoff_t end, struct sp_node *new)
@@ -3226,6 +3275,13 @@ static int shared_policy_replace(struct shared_policy *sp, pgoff_t start,
 		}
 	}
 	n = sp_lookup(sp, start, end);
+
+	ret = sp_check_node_limit(sp, n, start, end, new);
+	if (ret) {
+		write_unlock(&sp->lock);
+		goto err_out;
+	}
+
 	/* Take care of old policies in the same range. */
 	while (n && n->start < end) {
 		struct rb_node *next = rb_next(&n->nd);
@@ -3297,6 +3353,7 @@ void mpol_shared_policy_init(struct shared_policy *sp, struct mempolicy *mpol)
 
 	sp->root = RB_ROOT;		/* empty tree == default mempolicy */
 	rwlock_init(&sp->lock);
+	sp->nr_nodes = 0;
 
 	if (mpol) {
 		struct sp_node *sn;

-- 
2.43.0