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.

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

syscall: add Mmap for Solaris and Illumos? #52875

Closed
bcmills opened this issue May 12, 2022 · 5 comments
Closed

syscall: add Mmap for Solaris and Illumos? #52875

bcmills opened this issue May 12, 2022 · 5 comments
Assignees
@tklauser
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Milestone
Go1.20

Comments

@bcmills
Copy link
Contributor

bcmills commented May 12, 2022 •
edited
Loading

Solaris supports an mmap syscall.

x/sys/unix.Mmap is defined for GOOS=solaris, and runtime.mmap appears to use the system call as well.

And yet, even though Mmap exists on Solaris and syscall.Mmap exists on most Go Unix ports, for some reason syscall.Mmap does not appear to be defined for GOOS=solaris or GOOS=illumos.

We're looking at possibly using syscall.Mmap in cmd/go in https://go.dev/cl/403975 (CC @matloob). Would it make sense to add syscall.Mmap, syscall.Munmap, and related constants for Solaris and Illumos for consistency with other platforms?

(CC @ianlancetaylor @tklauser @golang/solaris @golang/illumos)
@bcmills bcmills added OS-Solaris NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 12, 2022
@bcmills bcmills added this to the Backlog milestone May 12, 2022
@ianlancetaylor
Copy link
Member

Although the syscall package is frozen, I think that this kind of addition is OK since it matches other targets.

@tklauser
Copy link
Member

tklauser commented May 13, 2022 •
edited
Loading

I too think this is OK.

There have been similar additions in the past when a particular function was present in syscall for some, but not all platforms. See e.g. https://go.dev/cl/403394, https://go.dev/cl/391434, https://go.dev/cl/390714 for some recent examples where this was the case for solaris and illumos.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/413374 mentions this issue: syscall: add Mmap and Munmap on solaris

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/413375 mentions this issue: cmd/go/internal/mmap: use syscall.Mmap on solaris

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
@prattmic prattmic moved this from Triage Backlog to In Progress in Go Compiler / Runtime Jul 27, 2022
@tklauser tklauser removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 28, 2022
Repository owner moved this from In Progress to Done in Go Compiler / Runtime Aug 9, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@tklauser @jproberts
syscall: add Mmap and Munmap on solaris
527343f 
They exist on all other Unix ports, define them on GOOS=solaris as well.

Fixes golang#52875

Change-Id: I7285156b3b48ce12fbcc6d1d88865540a5c51a21
Reviewed-on: https://go-review.googlesource.com/c/go/+/413374
Run-TryBot: Tobias Klauser <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Tobias Klauser <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
gopherbot pushed a commit that referenced this issue Aug 16, 2022
@tklauser
cmd/go/internal/mmap: use syscall.Mmap on solaris
b565476 
Now that syscall.Mmap is defined on solaris (see CL 413374), use it in
mmapFile like on other Unix ports.

For #52875

Change-Id: Ic5c5a84da8613f0c6dc947a52b7fcca50af43d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/413375
Run-TryBot: Tobias Klauser <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/430496 mentions this issue: cmd, syscall: use syscall.Mmap on solaris for Go ≥ 1.20

gopherbot pushed a commit that referenced this issue Sep 15, 2022
@tklauser @gopherbot
cmd, syscall: use syscall.Mmap on solaris for Go ≥ 1.20
7c32844 
CL 413374 added syscall.Mmap on solaris. Use it in cmd/compile and
cmd/link if the bootstrap toolchain is Go ≥ 1.20.

For #52875
For #54265

Change-Id: I9a0534bf97926eecf0c6f1f9218e855344ba158f
Reviewed-on: https://go-review.googlesource.com/c/go/+/430496
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Auto-Submit: Tobias Klauser <[email protected]>
Run-TryBot: Tobias Klauser <[email protected]>
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Jun 23, 2023
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 23, 2023
@golang golang locked and limited conversation to collaborators Jun 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@tklauser tklauser

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
5 participants
@tklauser @dmitshur @ianlancetaylor @bcmills @gopherbot