Re: [PATCH] mm: centralize and fix max map count limit checking
From: David Hildenbrand
Date: Thu Sep 04 2025 - 13:42:22 EST
On 04.09.25 19:33, Lorenzo Stoakes wrote:
On Thu, Sep 04, 2025 at 01:22:51PM -0400, Liam R. Howlett wrote:
diff --git a/mm/mremap.c b/mm/mremap.c
index e618a706aff5..793fad58302c 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1040,7 +1040,7 @@ static unsigned long prep_move_vma(struct vma_remap_struct *vrm)
* We'd prefer to avoid failure later on in do_munmap:
* which may split one vma into three before unmapping.
*/
- if (current->mm->map_count >= sysctl_max_map_count - 3)
+ if (exceeds_max_map_count(current->mm, 4))
return -ENOMEM;
In my version this would be:
if (map_count_capacity(current->mm) < 4)
return -ENOMEM;
Someone could write map_count_capacity(current->mm) <= 4 and reintroduce
what this is trying to solve. And with the way it is written in this
patch, someone could pass in the wrong number.
Right, but I think 'capacity' is pretty clear here, if the caller does something
silly then that's on them...
I'm not sure this is worth doing. There are places we allow the count
to go higher.
...But yeah, it's kinda borderline as to how useful this is.
I _do_ however like the 'put map count in one place statically' rather than
having a global, so a minimal version of this could be to just have a helper
function that gets the sysctl_max_map_count, e.g.:
if (current->mm->mmap_count >= max_map_count() - 3)
I enjoy seeing sysctl_max_map_count hidden. But map_count_capacity() is
even more readable, so I like it.
I don't complete like the "capacity" term, but I cannot think of
something better right now. Maybe something around "free" or
"remaining", not sure.
I also don't completely like "map_count" (I know, I know, we call it
like that in structures), because it reminds me of the mapcount ...
talking somehow about "vmas" would be quite clear.
Anyhow, just as an inspiration my 2 cents ...
--
Cheers
David / dhildenb