libgo patch committed: Fix madvise on systems with page size != 4096

libgo patch committed: Fix madvise on systems with page size != 4096

[Ian Lance Taylor]

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

This patch from Anton Blanchard fixes libgo to adjust to the system page
size when calling madvise.  Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline and 4.9 branch.

Ian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 953 bytes --]

diff -r 3a53301d24d7 libgo/runtime/mheap.c
--- a/libgo/runtime/mheap.c	Tue Apr 22 16:43:35 2014 -0700
+++ b/libgo/runtime/mheap.c	Thu Apr 24 21:18:35 2014 -0700
@@ -387,7 +387,7 @@
 static uintptr
 scavengelist(MSpan *list, uint64 now, uint64 limit)
 {
-	uintptr released, sumreleased;
+	uintptr released, sumreleased, start, end, pagesize;
 	MSpan *s;
 
 	if(runtime_MSpanList_IsEmpty(list))
@@ -400,7 +400,17 @@
 			mstats.heap_released += released;
 			sumreleased += released;
 			s->npreleased = s->npages;
-			runtime_SysUnused((void*)(s->start << PageShift), s->npages << PageShift);
+
+			start = s->start << PageShift;
+			end = start + (s->npages << PageShift);
+
+			// Round start up and end down to ensure we
+			// are acting on entire pages.
+			pagesize = getpagesize();
+			start = ROUND(start, pagesize);
+			end &= ~(pagesize - 1);
+			if(end > start)
+				runtime_SysUnused((void*)start, end - start);
 		}
 	}
 	return sumreleased;

Re: [gofrontend-dev] libgo patch committed: Fix madvise on systems with page size != 4096

[Michael Hudson-Doyle]

"'Ian Lance Taylor <iant@google.com>' via gofrontend-dev"
<gofrontend-dev@googlegroups.com> writes:

> This patch from Anton Blanchard fixes libgo to adjust to the system page
> size when calling madvise.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline and 4.9 branch.

Hi, I think this patch will make my Canonical colleagues very happy (and
me when I get around to building a 64k page arm64 kernel...).  It looks
to me like this is also a problem in the gc runtime library though --
should the patch be sent there too?  (Apologies if it already has and I
missed it, I did look though).

Cheers,
mwh

> Ian
>
> -- 
> You received this message because you are subscribed to the Google Groups "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to gofrontend-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
> diff -r 3a53301d24d7 libgo/runtime/mheap.c
> --- a/libgo/runtime/mheap.c	Tue Apr 22 16:43:35 2014 -0700
> +++ b/libgo/runtime/mheap.c	Thu Apr 24 21:18:35 2014 -0700
> @@ -387,7 +387,7 @@
>  static uintptr
>  scavengelist(MSpan *list, uint64 now, uint64 limit)
>  {
> -	uintptr released, sumreleased;
> +	uintptr released, sumreleased, start, end, pagesize;
>  	MSpan *s;
>  
>  	if(runtime_MSpanList_IsEmpty(list))
> @@ -400,7 +400,17 @@
>  			mstats.heap_released += released;
>  			sumreleased += released;
>  			s->npreleased = s->npages;
> -			runtime_SysUnused((void*)(s->start << PageShift), s->npages << PageShift);
> +
> +			start = s->start << PageShift;
> +			end = start + (s->npages << PageShift);
> +
> +			// Round start up and end down to ensure we
> +			// are acting on entire pages.
> +			pagesize = getpagesize();
> +			start = ROUND(start, pagesize);
> +			end &= ~(pagesize - 1);
> +			if(end > start)
> +				runtime_SysUnused((void*)start, end - start);
>  		}
>  	}
>  	return sumreleased;

Re: [gofrontend-dev] libgo patch committed: Fix madvise on systems with page size != 4096

[Ian Lance Taylor]

On Sun, Apr 27, 2014 at 2:45 PM, Michael Hudson-Doyle
<michael.hudson@linaro.org> wrote:
> "'Ian Lance Taylor <iant@google.com>' via gofrontend-dev"
> <gofrontend-dev@googlegroups.com> writes:
>
>> This patch from Anton Blanchard fixes libgo to adjust to the system page
>> size when calling madvise.  Bootstrapped and ran Go testsuite on
>> x86_64-unknown-linux-gnu.  Committed to mainline and 4.9 branch.
>
> Hi, I think this patch will make my Canonical colleagues very happy (and
> me when I get around to building a 64k page arm64 kernel...).  It looks
> to me like this is also a problem in the gc runtime library though --
> should the patch be sent there too?  (Apologies if it already has and I
> missed it, I did look though).

It's a potential problem in the gc runtime library, but I think it's
not a real problem at this point, since gc only runs on systems with
4k pages anyhow.

Ian