Try adding:

diff --git a/src/syscall/types_linux.go b/src/syscall/types_linux.go
index 9bccfca..3a105a4 100644
--- a/src/syscall/types_linux.go
+++ b/src/syscall/types_linux.go
@@ -117,6 +117,9 @@ struct my_epoll_event {
        // alignment requirements of EABI
        int32_t padFd;
 #endif
+#ifdef __powerpc64__
+       int32_t _pad;
+#endif
        int32_t fd;
        int32_t pad;
 };

And then regenerating it, then send a CL with both changes.

OK, this is what I tried, but didn't know I had to do something
explicitly to
regenerate the file. Do I just run mkall.sh with the right GOOS and GOARCH?

Also, this fixes the size but doesn't it matter that the members of the
structure don't really describe the fields correctly?

In runtime/defs_linux_ppc64le.go it looks right:

type epollevent struct {
events uint32
pad_cgo_0 [4]byte
data [8]byte // unaligned uintptr
}

On 04/05/2016 01:41 PM, Brad Fitzpatrick wrote:

Try adding:

diff --git a/src/syscall/types_linux.go b/src/syscall/types_linux.go
index 9bccfca..3a105a4 100644
--- a/src/syscall/types_linux.go
+++ b/src/syscall/types_linux.go
@@ -117,6 +117,9 @@ struct my_epoll_event {
// alignment requirements of EABI
int32_t padFd;
#endif
+#ifdef powerpc64

• int32_t _pad;
  +#endif
  int32_t fd;
  int32_t pad;
  };

And then regenerating it, then send a CL with both changes.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#15135 (comment)

Do I just run mkall.sh with the right GOOS and GOARCH?

Probably. Or whatever it says at the top of the file.

Also, this fixes the size but doesn't it matter that the members of the
structure don't really describe the fields correctly?

It's a C union. Can't do much better. Users can manipulate the two 32-bit fields as needed, using unsafe if necessary. It's consistent with what we do elsewhere.

I created this CL https://go-review.googlesource.com/#/c/21582/
One note is that when I tried to use mkall.sh it was flaky. I tried just running the script and it changed multiple files and even the only file I cared about had extra changes unrelated to my change in types_linux.go. In addition, it removed all the +build ppc64le tags in the generated z files which I believe must be there. So what I did was just manually update the one file that mattered.

I just realized I still need to add the ppc64 change.... will do that now.

Okay, we really need to figure out why the auto-generation isn't working then. Hand-editing the z* files is just asking for pain later.

/cc @minux @ianlancetaylor

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

If I just run mkall.sh like this:

GOOS=linux GOARCH=ppc64le ./mkall.sh from the syscall directory then there are changes made to these files:

    modified:   zerrors_linux_ppc64le.go
    modified:   zsyscall_linux_ppc64le.go
    modified:   zsysnum_linux_ppc64le.go
    modified:   ztypes_linux_ppc64le.go

Instead if I use -n to show me the commands, I think I only need this one:

GOARCH=ppc64le go tool cgo -godefs types_linux.go |go run mkpost.go >ztypes_linux_ppc64le.go

But if I run that one alone it adds more changes than just to the epoll struct and removes the build tag. I see that the mkpost.go change was recent so I tried without that and got a lot more changes to the file.

Would I get different changes to these files if I ran this on different distros?

Would I get different changes to these files if I ran this on different distros?

I think so. I saw quite a few changes moving between RHEL 7 and Ubuntu 16.04. In particular I think the names of the padding fields has a habit of changing and there may be constants added/removed/changed.

But if I run that one alone it adds more changes than just to the epoll struct and removes the build tag. I see that the mkpost.go change was recent so I tried without that and got a lot more changes to the file.

I don't think you should see any fewer changes because of mkpost.go. It should only do 'gofmt' on ppc64. It would be nice to add the build tag generation to it. I think these been added manually: https://go-review.googlesource.com/#/c/10113/

Here is patch showing the changes to ztypes_linux_ppc64le.go if I just pull latest master and run:
GOARCH=ppc64 go tool cgo -godefs types_linux.go |go run mkpost.go >ztypes_linux_ppc64le.go
without changing any files.

golang-mkall.txt

This was done on RHEL7.2 ppc64, but the same kind of patch shows up on Ubuntu 14.04 & 14.10 ppc64le. I see if I can try a later Ubuntu.

If I remove the pipe through mkpost.go, the differences are due to formatting.

The struct ptrace* {} declarations were added for s390x but are harmless because they don't get exported on ppc64.

The fields that are removed are of length 0... I guess technically they shouldn't be removed due to API promises, but I don't see what practical use they could have (marker for the end of the struct perhaps?).

@bradfitz I missed your earlier request to update the CLs with the regenerated files. I just did the update as you requested earlier but I did change the patch so it didn't remove the build tags.

@bradfitz In mkpost.go there is this code, only done for s390x:

            // Export the types of PtraceRegs fields.
            re := regexp.MustCompile("ptrace(Psw|Fpregs|Per)")
            s = re.ReplaceAllString(s, "Ptrace$1")

            // Replace padding fields inserted by cgo with blank identifiers.
            re = regexp.MustCompile("Pad_cgo[A-Za-z0-9_]*")
            s = re.ReplaceAllString(s, "_")

            // Replace other unwanted fields with blank identifiers.
            re = regexp.MustCompile("X_[A-Za-z0-9_]*")
            s = re.ReplaceAllString(s, "_")

            // Force the type of RawSockaddr.Data to [14]int8 to match
            // the existing gccgo API.
            re = regexp.MustCompile("(Data\\s+\\[14\\])uint8")
            s = re.ReplaceAllString(s, "${1}int8")

I believe you were asking in the CL to replace unwanted fields with _ as is done in the 3rd regexp above, so I can move that below the if test and then it will be done for all GOARCHes instead of just s390x. But what about the replacement of padding fields with _ as in the 2nd regexp above? Should that be done for all GOARCHes too?

FYI, I've just rebuilt go using Lynn's patch to see if it fixed a docker hang we were seeing (on ppc64le), and it did. 👍

I was out for a few days.... This is a high priority for us because it affects the upstream Docker CI build on ppc64le and would like to get the golang fix upstream so Ubuntu can pick it up. I have updated the mkpost.go change according to the earlier suggestion by @bradfitz. Let me know what else needs to be done so we can get this in ASAP. Thanks.

Based on Ian's latest comments I have reverted my patch back to the minimum change needed to ztypes_linux_ppc64le.go and ztypes_linux_ppc64.go to fix the EpollEvent structure on these GOARCHes along with the change in types_linux.go. Trying to use the file generation scripts result in changes unrelated to this problem and introduce incompatibilities. I can open a new issue for the file generation problem if that is what should be done.

(Sorry, just asked on the CL what you answered here.)

I don't really mind if you revert unwanted changes to the ztypes files, but I do want to ensure that the changes you do make are ones produced by mkall.sh. Otherwise you are introducing yet another problem to deal with. I'm OK with leaving the old ones alone, but not with introducing new ones.

Not quite. Let me change the ztypes_* files so they match what gets generated, revert what shouldn't be there, and retest on both to make sure it's all good.

Can this be backported to go1.6?

@laboger, no promises, but filed #15288 to consider it.

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