Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: addrspace_free bug causes large mmap to fail on mips64 #14297

Closed
eswierk opened this issue Feb 11, 2016 · 11 comments
Closed

runtime: addrspace_free bug causes large mmap to fail on mips64 #14297

eswierk opened this issue Feb 11, 2016 · 11 comments

Comments

@eswierk
Copy link

eswierk commented Feb 11, 2016

Testing the new mips64 support, I ran into out of memory errors whenever the heap grew beyond ~500 MB. I isolated the problem to an apparent bug in addrspace_free (https://github.com/golang/go/blob/53b6661/src/runtime/mem_linux.go#L34). mincore returns _ENOMEM, not -_ENOMEM, if the given page is unmapped. Removing the minus sign makes the problem go away.

I don't know if there are any platforms where the return value is actually negative. The equivalent function in the gccgo runtime also assumes a positive value.

@bradfitz bradfitz added this to the Go1.6Maybe milestone Feb 11, 2016
@bradfitz
Copy link
Contributor

/cc @ianlancetaylor

I tagged this Go1.6Maybe, if there's a tiny mips64-only fix. But it's not really a supported architecture yet, so it's probably a 1.7 issue.

@davecheney
Copy link
Contributor

@bradfitz this file is used by all linux implementations. I think this warrants investigation as it could affect supported linux platforms.

@bradfitz
Copy link
Contributor

Yes, that's used by all linux implementations. We can investigate, but if nobody else is having problems, we don't want to touch common code this late in 1.6 if we can help it. Maybe all other architectures correct for this elsewhere but mips64 doesn't. That's the tiny fix I was alluding to.

@davecheney
Copy link
Contributor

I'm concerned that with the exception of arm32, nothing is really has been memory constrained enough to trigger this error path.

@ianlancetaylor
Copy link
Contributor

On GNU/Linux, on 386, amd64, arm, and arm64, system calls return a negative errno value, in eax, rax, r0, r0, respectively. On mips64 and ppc64, they return a positive errno value, in r2 and r3, respectively.

In the runtime package, I think that the implementation of mmap on arm64 is wrong and the implementation of mincore on mips64 and ppc64 is wrong.

@davecheney
Copy link
Contributor

Thanks Ian. I wonder if the scavenger being disabled on arm64 and ppc64
systems has hidden this error so far. I'm assuming that if we don't use
madvise to return memory to the OS, mincore could be wrong and it wouldn't
matter -- just a wild guess.

What is the page size on mips64?

On Thu, 11 Feb 2016, 16:21 Ian Lance Taylor notifications@github.com
wrote:

On GNU/Linux, on 386, amd64, arm, and arm64, system calls return a
negative errno value, in eax, rax, r0, r0, respectively. On mips64 and
ppc64, they return a positive errno value, in r2 and r3, respectively.

In the runtime package, I think that the implementation of mmap on arm64
is wrong and the implementation of mincore on mips64 and ppc64 is wrong.


Reply to this email directly or view it on GitHub
#14297 (comment).

@eswierk
Copy link
Author

eswierk commented Feb 11, 2016

Adjustable I think, but normally 4k.
On Feb 10, 2016 9:34 PM, "Dave Cheney" notifications@github.com wrote:

Thanks Ian. I wonder if the scavenger being disabled on arm64 and ppc64
systems has hidden this error so far. I'm assuming that if we don't use
madvise to return memory to the OS, mincore could be wrong and it wouldn't
matter -- just a wild guess.

What is the page size on mips64?

On Thu, 11 Feb 2016, 16:21 Ian Lance Taylor notifications@github.com
wrote:

On GNU/Linux, on 386, amd64, arm, and arm64, system calls return a
negative errno value, in eax, rax, r0, r0, respectively. On mips64 and
ppc64, they return a positive errno value, in r2 and r3, respectively.

In the runtime package, I think that the implementation of mmap on arm64
is wrong and the implementation of mincore on mips64 and ppc64 is wrong.


Reply to this email directly or view it on GitHub
#14297 (comment).


Reply to this email directly or view it on GitHub
#14297 (comment).

@ianlancetaylor
Copy link
Contributor

@eswierk can you test https://golang.org/cl/19455?

@gopherbot
Copy link

CL https://golang.org/cl/19455 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/19457 mentions this issue.

@eswierk
Copy link
Author

eswierk commented Feb 11, 2016

I tested https://golang.org/cl/19455 on mips64 hardware and it didn't work. I added a comment with details.

gopherbot pushed a commit that referenced this issue Feb 11, 2016
Updates #14297

Change-Id: I6b5f5020af5efaaa71280bdeb2ff99785ee9b959
Reviewed-on: https://go-review.googlesource.com/19457
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Feb 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants