Skip to content

x/sys/unix: add Mremap #60409

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.

Sign up for GitHub

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
Closed
rsc opened this issue May 24, 2023 · 15 comments
Closed

x/sys/unix: add Mremap #60409

rsc opened this issue May 24, 2023 · 15 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge help wanted Proposal Proposal-Accepted
Milestone
Unreleased

Comments

@rsc
Copy link
Contributor

rsc commented May 24, 2023

syscall provides a type-safe Mmap and Munmap but is missing Mremap. We should add it.
See #58625 (comment).
@rsc rsc added this to Proposals May 24, 2023
@gopherbot gopherbot added this to the Proposal milestone May 24, 2023
@rsc rsc moved this to Incoming in Proposals May 24, 2023
@rsc rsc mentioned this issue May 24, 2023
Open
@rsc rsc moved this from Incoming to Active in Proposals May 24, 2023
@rsc
Copy link
Contributor Author

rsc commented May 24, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@mdempsky
Copy link
Contributor

mremap isn't in POSIX, so not all UNIXes support it. In particular, it looks like at least darwin and openbsd don't implement it.

@willfaught
Copy link
Contributor

I thought syscall was frozen? Don't these changes belong in x/sys now?

@ianlancetaylor
Copy link
Member

Yes, looks like this support is copied into x/sys/unix. You're right, that is what we should change. Thanks.

@ianlancetaylor ianlancetaylor changed the title proposal: syscall: add Mremap proposal: x/sys/unix: add Mremap May 24, 2023
@rsc
Copy link
Contributor Author

rsc commented May 25, 2023

We do use syscall.Mmap from within std, so it's OK to update if necessary. That said, std doesn't need Mremap so adding it in x/sys/unix instead is fine.

@rsc
Copy link
Contributor Author

rsc commented May 31, 2023

Marking as likely accept for x/sys/unix. We can change syscall if we need it for something in the main distribution but we probably don't need it otherwise.

@rsc rsc moved this from Active to Likely Accept in Proposals May 31, 2023
@rsc
Copy link
Contributor Author

rsc commented May 31, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor Author

rsc commented Jun 7, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals Jun 7, 2023
@rsc rsc changed the title proposal: x/sys/unix: add Mremap x/sys/unix: add Mremap Jun 7, 2023
@rsc rsc modified the milestones: Proposal, Backlog Jun 7, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 7, 2023
@anton-kuklin
Copy link
Contributor

anton-kuklin commented Jun 7, 2023 •
edited
Loading

I will work on this :)

@ianlancetaylor
Copy link
Member

@anton-kuklin Go for it. Thanks.

@anton-kuklin anton-kuklin mentioned this issue Jun 12, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/502715 mentions this issue: unix: add Mremap for linux

gopherbot pushed a commit to golang/sys that referenced this issue Jun 16, 2023
@anton-kuklin @gopherbot
unix: add Mremap for linux
e0c3b6e 
For golang/go#60409

Change-Id: I75a9732ee996f0aeb91599d80803f96ada468c27
GitHub-Last-Rev: c348b61
GitHub-Pull-Request: #164
Reviewed-on: https://go-review.googlesource.com/c/sys/+/502715
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
@anton-kuklin
Copy link
Contributor

Solved!
CC: @aaronbee

@anton-kuklin
Copy link
Contributor

I guess we can close this issue?

@ianlancetaylor
Copy link
Member

We now have support for Linux, but not yet for other operating systems that have a mremap system call.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/508397 mentions this issue: unix: add Mremap for netbsd

@github-project-automation github-project-automation bot moved this from Todo to Done in Go Compiler / Runtime Jul 13, 2023
@dmitshur dmitshur modified the milestones: Backlog, Unreleased Jul 17, 2023
@golang golang locked and limited conversation to collaborators Jul 16, 2024
@rsc rsc removed this from Proposals Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge help wanted Proposal Proposal-Accepted
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
8 participants
@mdempsky @willfaught @rsc @mknyszek @dmitshur @ianlancetaylor @gopherbot @anton-kuklin