Re: [PATCH] mm: memory migration: bug in touch_unmapped_address

Re: [PATCH] mm: memory migration: bug in touch_unmapped_address

[Marcelo Tosatti]

Good catch.

That was the reason for the migration cache problems you were seeing?

On Mon, Feb 28, 2005 at 11:36:43AM -0600, Ray Bryant wrote:
> Hirokazu,
> 
> The length field in the call to get_user_pages() from touch_unmapped_pages()
> is incorrectly specified in bytes, not pages.
> 
> As a result of this, if you use the migration code to migrate a page, then
> subsequent pages (that are not necessarily currently allocated or mapped)
> can be allocated and mapped as a result of the migration call.
> 
> [touch_unmapped_pages() is added by the memory migration code from the 
> memory
> hotplug patch so this is not currently part of the mainline kernel]
> 
> See attached patch for the fix.
> -- 
> Best Regards,
> Ray
> -----------------------------------------------
>                   Ray Bryant
> 512-453-9679 (work)         512-507-7807 (cell)
> raybry@sgi.com             raybry@austin.rr.com
> The box said: "Requires Windows 98 or better",
>            so I installed Linux.
> -----------------------------------------------

> The "len" parameter (4th arg) of get_user_pages() is in pages, not
> bytes.  The effect of this bug is that if you migrate a page, and
> if this page is followed by valid virtual addresses, but these 
> pages have not yet been touched and allocated, then the migration
> call will cause those pages to be touched and allocated.  The number
> of pages so effected is the min of (16384, the remaining number of
> pages in the vma, the number of pages required to fill out the
> current pmd).
> 
> Signed-off-by: Ray Bryant <raybry@sgi.com>
> 
> Index: linux/mm/rmap.c
> ===================================================================
> --- linux.orig/mm/rmap.c	2005-01-30 10:34:03.000000000 -0800
> +++ linux/mm/rmap.c	2005-02-28 08:53:30.000000000 -0800
> @@ -554,8 +554,7 @@ touch_unmapped_address(struct list_head 
>  		vma = find_vma(v1->mm, v1->addr);
>  		if (vma == NULL)
>  			goto out;
> -		error = get_user_pages(current, v1->mm, v1->addr, PAGE_SIZE,
> -					0, 0, NULL, NULL);
> +		error = get_user_pages(current, v1->mm, v1->addr, 1, 0, 0, NULL, NULL);
>  		if (error < 0)
>  			ret = error;
>  	out:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

[PATCH] mm: memory migration: bug in touch_unmapped_address

[Ray Bryant]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

Hirokazu,

The length field in the call to get_user_pages() from touch_unmapped_pages()
is incorrectly specified in bytes, not pages.

As a result of this, if you use the migration code to migrate a page, then
subsequent pages (that are not necessarily currently allocated or mapped)
can be allocated and mapped as a result of the migration call.

[touch_unmapped_pages() is added by the memory migration code from the memory
hotplug patch so this is not currently part of the mainline kernel]

See attached patch for the fix.
-- 
Best Regards,
Ray
-----------------------------------------------
                   Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
raybry@sgi.com             raybry@austin.rr.com
The box said: "Requires Windows 98 or better",
            so I installed Linux.
-----------------------------------------------

[-- Attachment #2: fix-len-param-in-touch_unmapped_address.patch --]
[-- Type: text/plain, Size: 1067 bytes --]

The "len" parameter (4th arg) of get_user_pages() is in pages, not
bytes.  The effect of this bug is that if you migrate a page, and
if this page is followed by valid virtual addresses, but these 
pages have not yet been touched and allocated, then the migration
call will cause those pages to be touched and allocated.  The number
of pages so effected is the min of (16384, the remaining number of
pages in the vma, the number of pages required to fill out the
current pmd).

Signed-off-by: Ray Bryant <raybry@sgi.com>

Index: linux/mm/rmap.c
===================================================================
--- linux.orig/mm/rmap.c	2005-01-30 10:34:03.000000000 -0800
+++ linux/mm/rmap.c	2005-02-28 08:53:30.000000000 -0800
@@ -554,8 +554,7 @@ touch_unmapped_address(struct list_head 
 		vma = find_vma(v1->mm, v1->addr);
 		if (vma == NULL)
 			goto out;
-		error = get_user_pages(current, v1->mm, v1->addr, PAGE_SIZE,
-					0, 0, NULL, NULL);
+		error = get_user_pages(current, v1->mm, v1->addr, 1, 0, 0, NULL, NULL);
 		if (error < 0)
 			ret = error;
 	out:

Re: [PATCH] mm: memory migration: bug in touch_unmapped_address

[Ray Bryant]

Marcelo Tosatti wrote:
> Good catch.
> 
> That was the reason for the migration cache problems you were seeing?
> 

AFAIK, no.  I found this problem because pages were being touched after
I scanned for pages to be migrated -- the result was that pages that
had not been touched by the application were being touched and then
left on the old nodes.

For the moment, I'm still pursuing a strategy of getting my manual
page migration code to work without the migration cache -- once I've
got that all working I'll come back and revisit the migration cache
bug I had reported.

-- 
Best Regards,
Ray
-----------------------------------------------
                   Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
raybry@sgi.com             raybry@austin.rr.com
The box said: "Requires Windows 98 or better",
            so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] mm: memory migration: bug in touch_unmapped_address

[Hirokazu Takahashi]

Hi Ray,

> Hirokazu,
> 
> The length field in the call to get_user_pages() from touch_unmapped_pages()
> is incorrectly specified in bytes, not pages.
>
> As a result of this, if you use the migration code to migrate a page, then
> subsequent pages (that are not necessarily currently allocated or mapped)
> can be allocated and mapped as a result of the migration call.

Yes, you're absolutely right.
I'll fix it soon.

> [touch_unmapped_pages() is added by the memory migration code from the memory
> hotplug patch so this is not currently part of the mainline kernel]
> 
> See attached patch for the fix.

Thank you,
Hirokazu Takahashi
   who is the person looking for new sponsors to help us to
   continue developing the memory migration feature.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>