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

runtime: drop NetBSD kernel bug sysmon workaround fixed in NetBSD 9.2 #46495

Open
prattmic opened this issue Jun 1, 2021 · 6 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD
Milestone

Comments

@prattmic
Copy link
Member

prattmic commented Jun 1, 2021

#42515 identified a NetBSD kernel bug (kern/50094) affecting the Go runtime. http://golang.org/cl/277332 introduced a (rather subpar) workaround for this issue.

@jdolecek-zz notes that the fix for this bug is included in NetBSD 9.2, making the workaround unnecessary.

Ideally we just drop http://golang.org/cl/277332, but this depends on our support policy for NetBSD. https://github.com/golang/go/wiki/NetBSD states that we currently support NetBSD 7+, but not what the official policy is. (OTOH, e.g., OpenBSD is explicitly supported only for the two most recent releases: https://github.com/golang/go/wiki/OpenBSD#longterm-support).

Based on my reading of https://www.netbsd.org/releases/formal.html, my interpretation would be that Go should support NetBSD 8 and 9 (and presumably drop 8 when 10 is released?). Do others agree? If so, we should probably add a version check to the workaround and keep it until NetBSD 9 is dropped.

cc @bsiegert @dmitshur @mknyszek

@dmitshur
Copy link
Contributor

dmitshur commented Jun 1, 2021

I agree we should find a support policy that automatically drops support for old NetBSD releases when new ones are made, similar to how OpenBSD has one.

CC @golang/release.

@tklauser
Copy link
Member

tklauser commented Jun 3, 2021

Another option to already disable the workaround for NetBSD 9.2 while still retaining compatibility with older NetBSD versions would be to check sysctlInt([]uint32{_CTL_KERN, _KERN_OSREV}) in runtime.osinit and disable the fix if that value is >= 902000000.

I've sent https://golang.org/cl/324472 implementing that.

@gopherbot
Copy link

Change https://golang.org/cl/324472 mentions this issue: runtime: skip sysmon workaround on NetBSD >= 9.2

@cherrymui cherrymui added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 7, 2021
@cherrymui cherrymui added this to the Backlog milestone Jun 7, 2021
gopherbot pushed a commit that referenced this issue Aug 16, 2021
Detect the NetBSD version in osinit and only enable the workaround for
the kernel bug identified in #42515 for NetBSD versions older than 9.2.

For #42515
For #46495

Change-Id: I808846c7f8e47e5f7cc0a2f869246f4bd90d8e22
Reviewed-on: https://go-review.googlesource.com/c/go/+/324472
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Trust: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@hzzb
Copy link
Contributor

hzzb commented Mar 9, 2022

Another option to already disable the workaround for NetBSD 9.2 while still retaining compatibility with older NetBSD versions would be to check sysctlInt([]uint32{_CTL_KERN, _KERN_OSREV}) in runtime.osinit and disable the fix if that value is >= 902000000.

I've sent https://golang.org/cl/324472 implementing that.

The CL you sent does os version checking in os_netbsd.go. It seems to me that no GOOS == "netbsd" rechecking needed in sysmon. Because needSysmonWorkaround would be true only on netbsd platform. right?

CC @tklauser

@tklauser
Copy link
Member

Another option to already disable the workaround for NetBSD 9.2 while still retaining compatibility with older NetBSD versions would be to check sysctlInt([]uint32{_CTL_KERN, _KERN_OSREV}) in runtime.osinit and disable the fix if that value is >= 902000000.
I've sent https://golang.org/cl/324472 implementing that.

The CL you sent does os version checking in os_netbsd.go. It seems to me that no GOOS == "netbsd" rechecking needed in sysmon. Because needSysmonWorkaround would be true only on netbsd platform. right?

CC @tklauser

Yes, needSysmonWorkaround will only be true on netbsd. However, the additional GOOS == "netbsd" check will lead the compiler to omit the entire branch on non-netbsd platforms, i.e. needSysmonWorkaround will not need to be checked at runtime.

@hzzb
Copy link
Contributor

hzzb commented Mar 10, 2022

Another option to already disable the workaround for NetBSD 9.2 while still retaining compatibility with older NetBSD versions would be to check sysctlInt([]uint32{_CTL_KERN, _KERN_OSREV}) in runtime.osinit and disable the fix if that value is >= 902000000.
I've sent https://golang.org/cl/324472 implementing that.

The CL you sent does os version checking in os_netbsd.go. It seems to me that no GOOS == "netbsd" rechecking needed in sysmon. Because needSysmonWorkaround would be true only on netbsd platform. right?
CC @tklauser

Yes, needSysmonWorkaround will only be true on netbsd. However, the additional GOOS == "netbsd" check will lead the compiler to omit the entire branch on non-netbsd platforms, i.e. needSysmonWorkaround will not need to be checked at runtime.

Got it. Thanks for your explanation.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-NetBSD
Projects
Status: Triage Backlog
Development

No branches or pull requests

6 participants