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

Jump to bottom

syscall: EpollEvent struct incorrect for s390x #16021

Closed
mundaym opened this issue Jun 9, 2016 · 4 comments
Closed

syscall: EpollEvent struct incorrect for s390x #16021

mundaym opened this issue Jun 9, 2016 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.7Maybe

Comments

@mundaym
Copy link
Member

mundaym commented Jun 9, 2016

This was fixed for ppc64/ppc64le in #15135. The struct is missing a padding field between the Events and Fd fields.

I think the syscall.EpollEvent struct is wrong for all 64-bit architectures except amd64 (which uses the packed attribute and therefore has no alignment padding) and ppc64/ppc64le (which was fixed).

https://github.com/docker/containerd currently uses the syscall.EpollEvent struct directly for some operations. Presumably they should be using golang.org/x/sys/unix (which uses the correct padding on arm64 and s390x) instead however I'd like to fix this anyway since it is a very small change.

CL to follow.
@ianlancetaylor ianlancetaylor added this to the Go1.7Maybe milestone Jun 9, 2016
@gopherbot
Copy link

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

@mwhudson
Copy link
Contributor

mwhudson commented Jun 9, 2016

Am I going to want to fix this on s390x (and arm64 I guess?) for the version of Go in Ubuntu 16.04 before I try to build docker/containerd there? Do you have a test case, or are containerd's own tests enough?

@mundaym
Copy link
Member Author

mundaym commented Jun 9, 2016

I think you will need to yes. You might also want to pick up 0324a3f if you do an update (this also came up during docker testing).

The issue manifests itself as docker-containerd hanging when trying to run the CI tests, so I guess you won't be able to get the test suite to start without this change.

I don't think you'll need the change for arm64 - they appear to have worked around this issue by re-implementing the epoll calls using cgo (!): docker-archive/containerd@6299489.

@mwhudson
Copy link
Contributor

Thanks, I've got both those fixes in 16.10 now and have started the process for getting them into 16.04.

mundaym added a commit to linux-on-ibm-z/go that referenced this issue Jun 17, 2016
@mundaym
syscall: add a padding field to EpollEvent on s390x
dfb69df 
Fixes golang#16021.

Change-Id: I55df38bbccd2641abcb54704115002a9aa04325d
Reviewed-on: https://go-review.googlesource.com/23962
Run-TryBot: Michael Munday <munday@ca.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jun 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7Maybe
Development

No branches or pull requests
4 participants
@mwhudson @ianlancetaylor @gopherbot @mundaym