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

x/sys/unix: Missing constants for fadvise() for Linux/arm #16816

Closed
fd0 opened this issue Aug 21, 2016 · 8 comments
Closed

x/sys/unix: Missing constants for fadvise() for Linux/arm #16816

fd0 opened this issue Aug 21, 2016 · 8 comments

Comments

@fd0
Copy link

fd0 commented Aug 21, 2016

This is a followup-issue for #15114, which added constants for the Fadvise() syscall for amd64 and 386 on Linux. I discovered that the constants are also missing for Linux/arm. For all the other architectures they are there, only arm is missing.

I've tried running mkall.sh but this fails:

$ GOOS=linux GOARCH=arm ./mkall.sh
gcc: error: unrecognized command line option '-marm'; did you mean '-mabm'?
gcc: error: unrecognized command line option '-marm'; did you mean '-mabm'?
fork/exec /tmp/go-build154582490/command-line-arguments/_obj/exe/mkpost: exec format error

Is mkall.sh supposed to be run on the target architecture?

I propose adding the constants FADV_* manually to ztypes_linux_arm.go, the constants are the same: https://github.com/thorvalds/linux/blob/master/include/uapi/linux/fadvise.h

Would a CL which adds the constants manually be accepted?

Could you please advise me how to add the constants for Linux on arm?

@fd0 fd0 changed the title x/sys/unix: Missing constants for fadvise() for Linux/non-x86 x/sys/unix: Missing constants for fadvise() for Linux/arm Aug 21, 2016
@bradfitz
Copy link
Contributor

Would a CL which adds the constants manually be accepted?

That adds maintenance pain for everybody in the future when your manual edits would keep getting auto-removed, and people would have to keep manually adding them back.

Related: #15282

/cc @ianlancetaylor

@bradfitz bradfitz added this to the Unreleased milestone Aug 21, 2016
@fd0
Copy link
Author

fd0 commented Aug 21, 2016

Ok, I understand. If there is anything I can do to help (at least anything that's not "refactor this all"), please let me know.

@bradfitz
Copy link
Contributor

Actually, let's just separate out the hand-written files. As long as you don't manually touch the machine-generated ones we should be fine.

Feel free to send CLs adding to or creating syscall_linux_arm64.go or whatever the manual files are.

@fd0
Copy link
Author

fd0 commented Aug 21, 2016

That would work for now, adding the constants manually to a new file types_linux_arm.go. But since types_linux.go contains the FADV_* constants, the next time mkall.sh is run for/on Linux/arm, they will also be written to ztypes_linux_arm.go and the code won't compile any more.

In order to mitigate this, the constants need to be removed from types_linux.go (the "template") and ztypes_*.go.

I'd really like to solve my problem here, but I don't that this is a good idea.

How is mkall.sh supposed to work? Is it required to execute it on the target platform?

If I understood @ianlancetaylor in #15114 (comment) correctly, rebuilding the machine generated files produces many unrelated changes (which confirms my observations).

@ianlancetaylor
Copy link
Contributor

You shouldn't need to change types_linux.go or add types_linux_arm.go. Just run mkall.sh on a ARM GNU/Linux system. Then manually revert the unrelated changes.

@fd0
Copy link
Author

fd0 commented Aug 22, 2016

Ok, I'll see if I can get access to some box on ARM.

@fd0
Copy link
Author

fd0 commented Oct 22, 2016

Submitted as https://go-review.googlesource.com/31641

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Oct 23, 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

4 participants