Skip to content

Commit 937582e

Browse files
lorenzo-stoakesakpm00
authored andcommitted
mm/mremap: correctly handle partial mremap() of VMA starting at 0
Patch series "refactor mremap and fix bug", v3. The existing mremap() logic has grown organically over a very long period of time, resulting in code that is in many parts, very difficult to follow and full of subtleties and sources of confusion. In addition, it is difficult to thread state through the operation correctly, as function arguments have expanded, some parameters are expected to be temporarily altered during the operation, others are intended to remain static and some can be overridden. This series completely refactors the mremap implementation, sensibly separating functions, adding comments to explain the more subtle aspects of the implementation and making use of small structs to thread state through everything. The reason for doing so is to lay the groundwork for planned future changes to the mremap logic, changes which require the ability to easily pass around state. Additionally, it would be unhelpful to add yet more logic to code that is already difficult to follow without first refactoring it like this. The first patch in this series additionally fixes a bug when a VMA with start address zero is partially remapped. Tested on real hardware under heavy workload and all self tests are passing. This patch (of 3): Consider the case of a partial mremap() (that results in a VMA split) of an accountable VMA (i.e. which has the VM_ACCOUNT flag set) whose start address is zero, with the MREMAP_MAYMOVE flag specified and a scenario where a move does in fact occur: addr end | | v v |-------------| | vma | |-------------| 0 This move is affected by unmapping the range [addr, end). In order to prevent an incorrect decrement of accounted memory which has already been determined, the mremap() code in move_vma() clears VM_ACCOUNT from the VMA prior to doing so, before reestablishing it in each of the VMAs post-split: addr end | | v v |---| |---| | A | | B | |---| |---| Commit 6b73cff ("mm: change munmap splitting order and move_vma()") changed this logic such as to determine whether there is a need to do so by establishing account_start and account_end and, in the instance where such an operation is required, assigning them to vma->vm_start and vma->vm_end. Later the code checks if the operation is required for 'A' referenced above thusly: if (account_start) { ... } However, if the VMA described above has vma->vm_start == 0, which is now assigned to account_start, this branch will not be executed. As a result, the VMA 'A' above will remain stripped of its VM_ACCOUNT flag, incorrectly. The fix is to simply convert these variables to booleans and set them as required. Link: https://lkml.kernel.org/r/cover.1741639347.git.lorenzo.stoakes@oracle.com Link: https://lkml.kernel.org/r/dc55cb6db25d97c3d9e460de4986a323fa959676.1741639347.git.lorenzo.stoakes@oracle.com Fixes: 6b73cff ("mm: change munmap splitting order and move_vma()") Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Reviewed-by: Harry Yoo <harry.yoo@oracle.com> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent aed877c commit 937582e

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

mm/mremap.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -705,8 +705,8 @@ static unsigned long move_vma(struct vm_area_struct *vma,
705705
unsigned long vm_flags = vma->vm_flags;
706706
unsigned long new_pgoff;
707707
unsigned long moved_len;
708-
unsigned long account_start = 0;
709-
unsigned long account_end = 0;
708+
bool account_start = false;
709+
bool account_end = false;
710710
unsigned long hiwater_vm;
711711
int err = 0;
712712
bool need_rmap_locks;
@@ -790,9 +790,9 @@ static unsigned long move_vma(struct vm_area_struct *vma,
790790
if (vm_flags & VM_ACCOUNT && !(flags & MREMAP_DONTUNMAP)) {
791791
vm_flags_clear(vma, VM_ACCOUNT);
792792
if (vma->vm_start < old_addr)
793-
account_start = vma->vm_start;
793+
account_start = true;
794794
if (vma->vm_end > old_addr + old_len)
795-
account_end = vma->vm_end;
795+
account_end = true;
796796
}
797797

798798
/*
@@ -832,7 +832,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
832832
/* OOM: unable to split vma, just get accounts right */
833833
if (vm_flags & VM_ACCOUNT && !(flags & MREMAP_DONTUNMAP))
834834
vm_acct_memory(old_len >> PAGE_SHIFT);
835-
account_start = account_end = 0;
835+
account_start = account_end = false;
836836
}
837837

838838
if (vm_flags & VM_LOCKED) {

0 commit comments

Comments
 (0)