--- ./mm/mremap.c.mmacct 2005-05-10 16:10:40.000000000 +0400
+++ ./mm/mremap.c 2005-05-10 18:12:13.000000000 +0400
@@ -213,6 +213,7 @@ static unsigned long move_vma(struct vm_
old_len = new_len;
old_addr = new_addr;
new_addr = -ENOMEM;
+ new_len = 0;
}
/* Conceal VM_ACCOUNT so old reservation is not undone */
On Wed, 11 May 2005, Kirill Korotaev wrote:
> Sorry, forgot to write that all these patches are against 2.6.12-rc4...
>
> This patch fixes mm->total_vm and mm->locked_vm acctounting in case
> when move_page_tables() fails inside move_vma().
>
> Signed-Off-By: Kirill Korotaev <[email protected]>
>
> --- ./mm/mremap.c.mmacct 2005-05-10 16:10:40.000000000 +0400
> +++ ./mm/mremap.c 2005-05-10 18:12:13.000000000 +0400
> @@ -213,6 +213,7 @@ static unsigned long move_vma(struct vm_
> old_len = new_len;
> old_addr = new_addr;
> new_addr = -ENOMEM;
> + new_len = 0;
> }
>
> /* Conceal VM_ACCOUNT so old reservation is not undone */
Are you sure?
The way it's supposed to work is that the do_munmap(,,old_len) which
follows, which normally unmaps the area moved from, when unsuccessful
unmaps the area moved to: which will "mistakenly" decrement total_vm etc.
by old_len, which needs to be bumped back up by that amount before leaving.
Hugh
--- ./mm/mremap.c.mmacct 2005-05-10 16:10:40.000000000 +0400
+++ ./mm/mremap.c 2005-05-12 11:56:17.000000000 +0400
@@ -224,6 +224,12 @@ static unsigned long move_vma(struct vm_
split = 1;
}
+ /*
+ * if we failed to move page tables we still do total_vm increment
+ * since do_munmap() will decrement it by old_len == new_len
+ */
+ mm->total_vm += new_len >> PAGE_SHIFT;
+
if (do_munmap(mm, old_addr, old_len) < 0) {
/* OOM: unable to split vma, just get accounts right */
vm_unacct_memory(excess >> PAGE_SHIFT);
@@ -237,7 +243,6 @@ static unsigned long move_vma(struct vm_
vma->vm_next->vm_flags |= VM_ACCOUNT;
}
- mm->total_vm += new_len >> PAGE_SHIFT;
__vm_stat_account(mm, vma->vm_flags, vma->vm_file, new_len>>PAGE_SHIFT);
if (vm_flags & VM_LOCKED) {
mm->locked_vm += new_len >> PAGE_SHIFT;
On Thu, 12 May 2005, Kirill Korotaev wrote:
>
> It would be nice to accept this small patch with a comment.
Adding such a comment is okay, but moving the total_vm += line
away from the __vm_stat_account and locked_vm += seems odd to me
- I'd rather you added your comment after the do_munmap(),
but no big deal (mmap_sem is held for writing throughout).
Hugh
--- ./mm/mremap.c.mmacct 2005-05-10 16:10:40.000000000 +0400
+++ ./mm/mremap.c 2005-05-12 11:56:17.000000000 +0400
@@ -224,6 +224,12 @@ static unsigned long move_vma(struct vm_
split = 1;
}
+ /*
+ * if we failed to move page tables we still do total_vm increment
+ * since do_munmap() will decrement it by old_len == new_len
+ */
+ mm->total_vm += new_len >> PAGE_SHIFT;
+
if (do_munmap(mm, old_addr, old_len) < 0) {
/* OOM: unable to split vma, just get accounts right */
vm_unacct_memory(excess >> PAGE_SHIFT);
@@ -237,7 +243,6 @@ static unsigned long move_vma(struct vm_
vma->vm_next->vm_flags |= VM_ACCOUNT;
}
- mm->total_vm += new_len >> PAGE_SHIFT;
__vm_stat_account(mm, vma->vm_flags, vma->vm_file, new_len>>PAGE_SHIFT);
if (vm_flags & VM_LOCKED) {
mm->locked_vm += new_len >> PAGE_SHIFT;