From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Liam R . Howlett" <Liam.Howlett@oracle.com>,
Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Peter Xu <peterx@redhat.com>
Subject: [PATCH hotfix 6.12 8/8] mm: do not attempt second merge for file-backed VMAs
Date: Tue, 22 Oct 2024 21:40:59 +0100 [thread overview]
Message-ID: <7b6bf6165080505feb5e00b313d2c84c25015e45.1729628198.git.lorenzo.stoakes@oracle.com> (raw)
In-Reply-To: <cover.1729628198.git.lorenzo.stoakes@oracle.com>
Previously, we'd always try to merge a file-backed VMA if its flags were
changed by the driver.
This however is rarely meaningful as typically the flags would be changed
to VM_PFNMAP or other VM_SPECIAL flags which are inherently unmergable.
In cases where it is meaningful (for instance DAX) it is doubtful that this
optimisation is worth the effort and maintenance risk of having to unwind
state and perform a merge.
Since we've observed bugs and resource leaks due to complexity in this
area, it is simply not acceptable to have a 'nice to have' optimisation
like this complicating an already very complicated code path, so let's
simply eliminate it.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
mm/vma.c | 39 ++-------------------------------------
1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/mm/vma.c b/mm/vma.c
index a271e2b406ab..fe1fe5099e78 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2260,8 +2260,7 @@ static int __mmap_prepare(struct mmap_state *map)
return 0;
}
-static int __mmap_new_file_vma(struct mmap_state *map, struct vm_area_struct *vma,
- struct vm_area_struct **mergep)
+static int __mmap_new_file_vma(struct mmap_state *map, struct vm_area_struct *vma)
{
struct vma_iterator *vmi = map->vmi;
struct vma_merge_struct *vmg = map->vmg;
@@ -2291,34 +2290,6 @@ static int __mmap_new_file_vma(struct mmap_state *map, struct vm_area_struct *vm
(vma->vm_flags & VM_MAYWRITE));
vma_iter_config(vmi, vmg->start, vmg->end);
- /*
- * If flags changed after mmap_file(), we should try merge
- * vma again as we may succeed this time.
- */
- if (unlikely(map->flags != vma->vm_flags && vmg->prev)) {
- struct vm_area_struct *merge;
-
- vmg->flags = vma->vm_flags;
- /* If this fails, state is reset ready for a reattempt. */
- merge = vma_merge_new_range(vmg);
-
- if (merge) {
- /*
- * ->mmap() can change vma->vm_file and fput
- * the original file. So fput the vma->vm_file
- * here or we would add an extra fput for file
- * and cause general protection fault
- * ultimately.
- */
- fput(vma->vm_file);
- vm_area_free(vma);
- vma_iter_free(vmi);
- *mergep = merge;
- } else {
- vma_iter_config(vmi, vmg->start, vmg->end);
- }
- }
-
map->flags = vma->vm_flags;
return 0;
}
@@ -2341,7 +2312,6 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
{
struct vma_iterator *vmi = map->vmi;
struct vma_merge_struct *vmg = map->vmg;
- struct vm_area_struct *merge = NULL;
int error = 0;
struct vm_area_struct *vma;
@@ -2365,7 +2335,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
}
if (vmg->file)
- error = __mmap_new_file_vma(map, vma, &merge);
+ error = __mmap_new_file_vma(map, vma);
else if (map->flags & VM_SHARED)
error = shmem_zero_setup(vma);
else
@@ -2374,9 +2344,6 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
if (error)
goto free_iter_vma;
- if (merge)
- goto file_expanded;
-
#ifdef CONFIG_SPARC64
/* TODO: Fix SPARC ADI! */
WARN_ON_ONCE(!arch_validate_flags(map->flags));
@@ -2393,8 +2360,6 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
* call covers the non-merge case.
*/
khugepaged_enter_vma(vma, map->flags);
-
-file_expanded:
ksm_add_vma(vma);
*vmap = vma;
--
2.47.0
next prev parent reply other threads:[~2024-10-22 20:41 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 20:40 [PATCH hotfix 6.12 0/8] fix error handling in mmap_region() and refactor Lorenzo Stoakes
2024-10-22 20:40 ` [PATCH hotfix 6.12 1/8] mm: avoid unsafe VMA hook invocation when error arises on mmap hook Lorenzo Stoakes
2024-10-22 21:14 ` Jann Horn
2024-10-23 16:56 ` Lorenzo Stoakes
2024-10-23 9:11 ` Vlastimil Babka
2024-10-23 14:22 ` Liam R. Howlett
2024-10-22 20:40 ` [PATCH hotfix 6.12 2/8] mm: unconditionally close VMAs on error Lorenzo Stoakes
2024-10-22 21:15 ` Jann Horn
2024-10-23 17:00 ` Lorenzo Stoakes
2024-10-23 9:24 ` Vlastimil Babka
2024-10-23 16:58 ` Lorenzo Stoakes
2024-10-23 14:26 ` Liam R. Howlett
2024-10-23 14:41 ` Liam R. Howlett
2024-10-22 20:40 ` [PATCH hotfix 6.12 3/8] mm: refactor map_deny_write_exec() Lorenzo Stoakes
2024-10-22 21:15 ` Jann Horn
2024-10-23 16:35 ` Lorenzo Stoakes
2024-10-23 9:57 ` Vlastimil Babka
2024-10-23 14:30 ` Liam R. Howlett
2024-10-23 16:25 ` Linus Torvalds
2024-10-23 16:37 ` Lorenzo Stoakes
2024-10-22 20:40 ` [PATCH hotfix 6.12 4/8] mm: resolve faulty mmap_region() error path behaviour Lorenzo Stoakes
2024-10-23 12:58 ` Vlastimil Babka
2024-10-23 14:20 ` Liam R. Howlett
2024-10-23 17:11 ` Lorenzo Stoakes
2024-10-23 15:02 ` Liam R. Howlett
2024-10-22 20:40 ` [PATCH hotfix 6.12 5/8] tools: testing: add additional vma_internal.h stubs Lorenzo Stoakes
2024-10-22 20:40 ` [PATCH hotfix 6.12 6/8] mm: insolate mmap internal logic to mm/vma.c Lorenzo Stoakes
2024-10-22 20:40 ` [PATCH hotfix 6.12 7/8] mm: refactor __mmap_region() Lorenzo Stoakes
2024-10-23 14:38 ` Vlastimil Babka
2024-10-23 15:21 ` Liam R. Howlett
2024-10-23 17:39 ` Lorenzo Stoakes
2024-10-23 18:12 ` Liam R. Howlett
2024-10-23 17:30 ` Lorenzo Stoakes
2024-10-23 17:19 ` Liam R. Howlett
2024-10-23 17:52 ` Lorenzo Stoakes
2024-10-22 20:40 ` Lorenzo Stoakes [this message]
2024-10-23 15:01 ` [PATCH hotfix 6.12 8/8] mm: do not attempt second merge for file-backed VMAs Vlastimil Babka
2024-10-23 15:16 ` Lorenzo Stoakes
2024-10-23 18:16 ` Liam R. Howlett
2024-10-23 18:20 ` Lorenzo Stoakes
2024-10-22 20:58 ` [PATCH hotfix 6.12 0/8] fix error handling in mmap_region() and refactor Lorenzo Stoakes
2024-10-23 8:37 ` Vlastimil Babka
2024-10-23 8:45 ` Lorenzo Stoakes
2024-10-23 10:22 ` Andrew Morton
2024-10-23 10:25 ` Andrew Morton
2024-10-23 10:26 ` Lorenzo Stoakes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7b6bf6165080505feb5e00b313d2c84c25015e45.1729628198.git.lorenzo.stoakes@oracle.com \
--to=lorenzo.stoakes@oracle.com \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=peterx@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox