| Age | Commit message (Collapse) | Author |
|---|
|
As mentioned in CL 584598, linkname is a mechanism that, when
abused, can break API integrity and even safety of Go programs.
CL 584598 is a first step to restrict the use of linknames, by
implementing a blocklist. This CL takes a step further, tightening
up the restriction by allowing linkname references ("pull") only
when the definition side explicitly opts into it, by having a
linkname on the definition (possibly to itself). This way, it is at
least clear on the definition side that the symbol, despite being
unexported, is accessed outside of the package. Unexported symbols
without linkname can now be actually private. This is similar to
the symbol visibility rule used by gccgo for years (which defines
unexported non-linknamed symbols as C static symbols).
As there can be pull-only linknames in the wild that may be broken
by this change, we currently only enforce this rule for symbols
defined in the standard library. Push linknames are added in the
standard library to allow things build.
Linkname references to external (non-Go) symbols are still allowed,
as their visibility is controlled by the C symbol visibility rules
and enforced by the C (static or dynamic) linker.
Assembly symbols are treated similar to linknamed symbols.
This is controlled by -checklinkname linker flag, currently not
enabled by default. A follow-up CL will enable it by default.
Change-Id: I07344f5c7a02124dbbef0fbc8fec3b666a4b2b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/585358
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
|
|
During my research of pahole with Go structs, I've found couple of
structs in runtime/ pkg where we can reduce several structs' sizes
highligted by pahole tool which detect byte holes and paddings.
Overall, there are 80 bytes reduced.
Change-Id: I398e5ed6f5b199394307741981cb5ad5b875e98f
Reviewed-on: https://go-review.googlesource.com/c/go/+/578795
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Joedian Reid <joedian@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
For #65355
Change-Id: I65dd090fb99de9b231af2112c5ccb0eb635db2be
Reviewed-on: https://go-review.googlesource.com/c/go/+/560155
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ibrahim Bazoka <ibrahimbazoka729@gmail.com>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
|
|
From the beginning of Go, the time package has had a gotcha:
if you use a select on <-time.After(1*time.Minute), even if the select
finishes immediately because some other case is ready, the underlying
timer from time.After keeps running until the minute is over. This
pins the timer in the timer heap, which keeps it from being garbage
collected and in extreme cases also slows down timer operations.
The lack of garbage collection is the more important problem.
The docs for After warn against this scenario and suggest using
NewTimer with a call to Stop after the select instead, purely to work
around this garbage collection problem.
Oddly, the docs for NewTimer and NewTicker do not mention this
problem, but they have the same issue: they cannot be collected until
either they are Stopped or, in the case of Timer, the timer expires.
(Tickers repeat, so they never expire.) People have built up a shared
knowledge that timers and tickers need to defer t.Stop even though the
docs do not mention this (it is somewhat implied by the After docs).
This CL fixes the garbage collection problem, so that a timer that is
unreferenced can be GC'ed immediately, even if it is still running.
The approach is to only insert the timer into the heap when some
channel operation is blocked on it; the last channel operation to stop
using the timer takes it back out of the heap. When a timer's channel
is no longer referenced, there are no channel operations blocked on
it, so it's not in the heap, so it can be GC'ed immediately.
This CL adds an undocumented GODEBUG asynctimerchan=1
that will disable the change. The documentation happens in
the CL 568341.
Fixes #8898.
Fixes #61542.
Change-Id: Ieb303b6de1fb3527d3256135151a9e983f3c27e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/512355
Reviewed-by: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
|
|
The timers had evolved to the point where the state was stored as follows:
if timer in heap:
state has timerHeaped set
if heap timer is stale:
heap deadline in t.when
real deadline in t.nextWhen
state has timerNextWhen set
else:
real deadline in t.when
t.nextWhen unset
else:
real deadline in t.when
t.nextWhen unset
That made it hard to find the real deadline and just hard to think about everything.
The new state is:
real deadline in t.when (always)
if timer in heap:
state has timerHeaped set
heap deadline in t.whenHeap
if heap timer is stale:
state has timerModified set
Separately, the 'state' word itself was being used as a lock
and state bits because the code started with CAS loops,
which we abstracted into the lock/unlock methods step by step.
At this point, we can switch to a real lock, making sure to
publish the one boolean needed by timers fast paths
at each unlock.
All this simplifies various logic considerably.
Change-Id: I35766204f7a26d999206bd56cc0db60ad1b17cbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/570335
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8753622336585847105/+/u/step/11/log/2
shows a staticlockranking crash with pollcache (defaulted to LEAF)
being held during a write barrier, which got unlucky and acquired
wbufSpans, triggering a lock ordering throw.
My change in https://go-review.googlesource.com/c/go/+/570335/13/src/runtime/netpoll.go
around line 700 caused batching of many write barriers on the first
call rather than having just a few write barriers on each call,
making the crash much more likely, but the ordering problem
appears to have always existed. We just never allocated enough
pollDescs to trigger it.
Change-Id: Icb5e8340a5027dd4f7535a5ef02b2868476539e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/571195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Continuing conversion from C to Go, change timer API to use methods.
[This is one CL in a refactoring stack making very small changes
in each step, so that any subtle bugs that we miss can be more
easily pinpointed to a small change.]
Change-Id: I4cb88a366993a77aa4fad739793a7db7213cc38c
Reviewed-on: https://go-review.googlesource.com/c/go/+/564131
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
The runtime was adjusting netpollWaiters before the waiting
goroutines were marked as ready. This could cause the scheduler
to report a deadlock because there were no goroutines ready to run.
Keeping netpollWaiters non-zero ensures that at least one goroutine
will call netpoll(-1) from findRunnable.
This does mean that if a program has network activity for a while
and then never has it again, and also has no timers, then we can leave
an M stranded in a call to netpoll from which it will never return.
At least this won't be a common case. And it's not new; this has been
a potential problem for some time.
Fixes #61454
Change-Id: I17c7f891c2bb1262fda12c6929664e64686463c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/511455
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
|
|
We used to decrement it in netpollgoready, but that missed
the common case of a descriptor becoming ready due to I/O.
All calls to netpollgoready go through netpollunblock,
so this shouldn't miss any decrements we missed before.
Fixes #60782
Change-Id: Ideefefa1ac96ca38e09fe2dd5d595c5dd7883237
Reviewed-on: https://go-review.googlesource.com/c/go/+/503923
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
Implements netpoll using WASI's poll_oneoff system call.
This enables non-blocking I/O support for wasip1.
Change-Id: Ie395fa49d651c8b8262d485e2847dd65b0a10bc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/493357
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Julien Fabre <ju.pryz@gmail.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
|
|
This change adds traceBlockReason which leaks fewer implementation
details of the tracer to the runtime. Currently, gopark is called with
an explicit trace event, but this leaks details about trace internals
throughout the runtime.
This change will make it easier to change out the trace implementation.
Change-Id: Id633e1704d2c8838c6abd1214d9695537c4ac7db
Reviewed-on: https://go-review.googlesource.com/c/go/+/494185
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
|
|
I think there is a theoretical possibility of a mistake before this CL.
pollCache.free would increment fdseq, but would not update atomicInfo.
The epoll code could compare to fdseq before the increment, but suspend
before calling setEventErr. The pollCache could get reallocated,
and pollOpen could clear eventErr. Then the setEventErr could continue
and set it again. Then pollOpen could call publishInfo.
Avoid this rather remote possibility by calling publishInfo after
incrementing fdseq. That ensures that delayed setEventErr will not
modify the eventErr flag.
Fixes #60133
Change-Id: I69e336535312544690821c9fd53f3023ff15b80c
Reviewed-on: https://go-review.googlesource.com/c/go/+/495297
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
It is possible for a netpoll file to be closed and for the pollDesc
to be reused while a netpoll is running. This normally only causes
spurious wakeups, but if there is an error on the old file then the
new file can be incorrectly marked as having an error.
Fix this problem on most systems by introducing an fd sequence field
and using that as a tag in a taggedPointer. The taggedPointer is
stored in epoll or kqueue or whatever is being used. If the taggedPointer
returned by the kernel has a tag that does not match the fd
sequence field, the notification is for a closed file, and we
can ignore it. We check the tag stored in the pollDesc, and we also
check the tag stored in the pollDesc.atomicInfo.
This approach does not work on 32-bit systems where the kernel
only provides a 32-bit field to hold a user value. On those systems
we continue to use the older method without the sequence protection.
This is not ideal, but it is not an issue on Linux because the kernel
provides a 64-bit field, and it is not an issue on Windows because
there are no poller errors on Windows. It is potentially an issue
on *BSD systems, but on those systems we already call fstat in newFile
in os/file_unix.go to avoid adding non-pollable files to kqueue.
So we currently don't know of any cases that will fail.
Fixes #59545
Change-Id: I9a61e20dc39b4266a7a2978fc16446567fe683ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/484837
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Orlando Labao <orlando.labao43@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org>
|
|
Implements OS interactions and memory management.
For #58141
Co-authored-by: Richard Musiol <neelance@gmail.com>
Co-authored-by: Achille Roussel <achille.roussel@gmail.com>
Co-authored-by: Julien Fabre <ju.pryz@gmail.com>
Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I876e7b033090c2fe2d76d2535bb63d52efa36185
Reviewed-on: https://go-review.googlesource.com/c/go/+/479618
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
|
|
Updates #53821
Change-Id: I8776382b3eb0b7752cfc0d9287b707039d3f05c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/425358
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: hopehook <hopehook@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Use explicit name pdNil for nil semaphore of a pollDesc to make it self-explanatory like pdReady and pdWait.
Change-Id: Ibfb246e14419d366edadbccac4d3717f0c135cb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/424923
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
|
|
Change-Id: Ia00acf248f3498d75e2451548f82d3c57cfed06f
Reviewed-on: https://go-review.googlesource.com/c/go/+/424995
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Updates #46731
Change-Id: Ic2208c8bb639aa1e390be0d62e2bd799ecf20654
Reviewed-on: https://go-review.googlesource.com/c/go/+/421878
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Updates #53821
Change-Id: Ifa2e5f5d4047117b1887c1e56851355547bb4f33
Reviewed-on: https://go-review.googlesource.com/c/go/+/423881
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
[This CL is part of a sequence implementing the proposal #51082.
The design doc is at https://go.dev/s/godocfmt-design.]
Run the updated gofmt, which reformats doc comments,
on the main repository. Vendored files are excluded.
For #51082.
Change-Id: I7332f099b60f716295fb34719c98c04eb1a85407
Reviewed-on: https://go-review.googlesource.com/c/go/+/384268
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
A future change to gofmt will rewrite
// Doc comment.
//go:foo
to
// Doc comment.
//
//go:foo
Apply that change preemptively to all comments (not necessarily just doc comments).
For #51082.
Change-Id: Iffe0285418d1e79d34526af3520b415a12203ca9
Reviewed-on: https://go-review.googlesource.com/c/go/+/384260
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
A run of lines that are indented with any number of spaces or tabs
format as a <pre> block. This commit fixes various doc comments
that format badly according to that (standard) rule.
For example, consider:
// - List item.
// Second line.
// - Another item.
Because the - lines are unindented, this is actually two paragraphs
separated by a one-line <pre> block. This CL rewrites it to:
// - List item.
// Second line.
// - Another item.
Today, that will format as a single <pre> block.
In a future release, we hope to format it as a bulleted list.
Various other minor fixes as well, all in preparation for reformatting.
For #51082.
Change-Id: I95cf06040d4186830e571cd50148be3bf8daf189
Reviewed-on: https://go-review.googlesource.com/c/go/+/384257
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
For #20322
For #51572
Change-Id: Id0b4799d097d01128e98ba4cc0092298357bca45
Reviewed-on: https://go-review.googlesource.com/c/go/+/389935
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
|
|
The netpoll code was written long ago, when the
only multiprocessors that Go ran on were x86.
It assumed that an atomic store would trigger a
full memory barrier and then used that barrier
to order otherwise racy access to a handful of fields,
including pollDesc.closing.
On ARM64, this code has finally failed, because
the atomic store is on a value completely unrelated
to any of the racily-accessed fields, and the ARMv8
hardware, unlike x86, is clever enough not to do a
full memory barrier for a simple atomic store.
We are seeing a constant background rate of trybot
failures where the net/http tests deadlock - a netpollblock
has clearly happened after the pollDesc has begun to close.
The code that does the racy reads is netpollcheckerr,
which needs to be able to run without acquiring a lock.
This CL fixes the race, without introducing unnecessary
inefficiency or deadlock, by arranging for every updater
of the relevant fields to publish a summary as a single
atomic uint32, and then having netpollcheckerr use a
single atomic load to fetch the relevant bits and then
proceed as before.
Fixes #45211 (until proven otherwise!).
Change-Id: Ib6788c8da4d00b7bda84d55ca3fdffb5a64c1a0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/378234
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes #49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
When these packages are released as part of Go 1.18,
Go 1.16 will no longer be supported, so we can remove
the +build tags in these files.
Ran go fix -fix=buildtag std cmd and then reverted the bootstrapDirs
as defined in src/cmd/dist/buildtool.go, which need to continue
to build with Go 1.4 for now.
Also reverted src/vendor and src/cmd/vendor, which will need
to be updated in their own repos first.
Manual changes in runtime/pprof/mprof_test.go to adjust line numbers.
For #41184.
Change-Id: Ic0f93f7091295b6abc76ed5cd6e6746e1280861e
Reviewed-on: https://go-review.googlesource.com/c/go/+/344955
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
|
|
Both netpollblock and netpollunblock read gpp using a non-atomic load.
When consuming a ready event, netpollblock clears gpp using a non-atomic
store, thus skipping a barrier.
Thus on systems with weak memory ordering, a sequence like so this is
possible:
T1 T2
1. netpollblock: read gpp -> pdReady
2. netpollblock: store gpp -> 0
3. netpollunblock: read gpp -> pdReady
4. netpollunblock: return
i.e., without a happens-before edge between (2) and (3), netpollunblock
may read the stale value of gpp.
Switch these access to use atomic loads and stores in order to create
these edges.
For ease of future maintainance, I've simply changed rg and wg to always
be accessed atomically, though I don't believe pollOpen or pollClose
require atomics today.
Fixes #48925
Change-Id: I903ea667eea320277610b4f969129935731520c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/355952
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
Change-Id: I95e91ff21420e396aef876e77bc4ccdc45ab40ca
GitHub-Last-Rev: 8e6bd3f002b1c29fed8ce1bd344f7727e8580555
GitHub-Pull-Request: golang/go#47372
Reviewed-on: https://go-review.googlesource.com/c/go/+/337249
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
|
|
Document that network poller implementations need to define
netpollclose.
Change-Id: Idc73dea7cfd503d4de7e1d95902f0f102cf5ed8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/297809
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
When netpollopen in poll_runtime_pollOpen returns an error, the work in
runtime_pollUnblock and runtime_pollClose can be avoided since the
underlying system call to set up the poller failed.
E.g. on linux, this avoids calling netpollclose and thus epoll_ctl(fd,
EPOLL_CTL_DEL, ...) in case the file does not support epoll, i.e.
epoll_ctl(fd, EPOLL_CTL_ADD, ...) in netpollopen failed.
Fixes #44552
Change-Id: I564d90340fd1ab3a6490526353616a447ae0cfb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/297392
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Make all our package sources use Go 1.17 gofmt format
(adding //go:build lines).
Part of //go:build change (#41184).
See https://golang.org/design/draft-gobuild
Change-Id: Ia0534360e4957e58cd9a18429c39d0e32a6addb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/294430
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
pointers to go:notinheap types should be treated as scalars. That
means they shouldn't be stored directly in interfaces, or directly
in reflect.Value.ptr.
Also be sure to use uintpr to compare such pointers in reflect.DeepEqual.
Fixes #42076
Change-Id: I53735f6d434e9c3108d4940bd1bae14c61ef2a74
Reviewed-on: https://go-review.googlesource.com/c/go/+/264480
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
I took some of the infrastructure from Austin's lock logging CR
https://go-review.googlesource.com/c/go/+/192704 (with deadlock
detection from the logs), and developed a setup to give static lock
ranking for runtime locks.
Static lock ranking establishes a documented total ordering among locks,
and then reports an error if the total order is violated. This can
happen if a deadlock happens (by acquiring a sequence of locks in
different orders), or if just one side of a possible deadlock happens.
Lock ordering deadlocks cannot happen as long as the lock ordering is
followed.
Along the way, I found a deadlock involving the new timer code, which Ian fixed
via https://go-review.googlesource.com/c/go/+/207348, as well as two other
potential deadlocks.
See the constants at the top of runtime/lockrank.go to show the static
lock ranking that I ended up with, along with some comments. This is
great documentation of the current intended lock ordering when acquiring
multiple locks in the runtime.
I also added an array lockPartialOrder[] which shows and enforces the
current partial ordering among locks (which is embedded within the total
ordering). This is more specific about the dependencies among locks.
I don't try to check the ranking within a lock class with multiple locks
that can be acquired at the same time (i.e. check the ranking when
multiple hchan locks are acquired).
Currently, I am doing a lockInit() call to set the lock rank of most
locks. Any lock that is not otherwise initialized is assumed to be a
leaf lock (a very high rank lock), so that eliminates the need to do
anything for a bunch of locks (including all architecture-dependent
locks). For two locks, root.lock and notifyList.lock (only in the
runtime/sema.go file), it is not as easy to do lock initialization, so
instead, I am passing the lock rank with the lock calls.
For Windows compilation, I needed to increase the StackGuard size from
896 to 928 because of the new lock-rank checking functions.
Checking of the static lock ranking is enabled by setting
GOEXPERIMENT=staticlockranking before doing a run.
To make sure that the static lock ranking code has no overhead in memory
or CPU when not enabled by GOEXPERIMENT, I changed 'go build/install' so
that it defines a build tag (with the same name) whenever any experiment
has been baked into the toolchain (by checking Expstring()). This allows
me to avoid increasing the size of the 'mutex' type when static lock
ranking is not enabled.
Fixes #38029
Change-Id: I154217ff307c47051f8dae9c2a03b53081acd83a
Reviewed-on: https://go-review.googlesource.com/c/go/+/207619
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Use explicit names for the error code returned by pollReset
and pollWait, rather than just 0, 1, 2, 3.
Change-Id: I0ab12cae57693deab7cca9cdd2fadd597e23a956
Reviewed-on: https://go-review.googlesource.com/c/go/+/226537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
Change-Id: I58ac10013cadd78618124cb7ff134384d158ea4f
GitHub-Last-Rev: 2dfff0d3d3d18ecb196d5357cdfec196424d9e3b
GitHub-Pull-Request: golang/go#36276
Reviewed-on: https://go-review.googlesource.com/c/go/+/212557
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
In Go 1.4 we renamed READY to pdReady and WAIT to pdWait as part of
rewriting netpoll from C to Go. Finish updating the comments to use
the new names.
Change-Id: I6cefc698b46c58211fd6be1489bdd70419454962
Reviewed-on: https://go-review.googlesource.com/c/go/+/223998
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
As a side-effect ensure that netpollinited only reports true when
netpoll initialization is complete.
Fixes #35282
Updates #35353
Change-Id: I21f08a04fcf229e0de5e6b5ad89c990426ae9b89
Reviewed-on: https://go-review.googlesource.com/c/go/+/204937
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
When we add a timer, make sure that the network poller is initialized,
since we will use it if we have to wait for the timer to be ready.
Updates #27707
Change-Id: I0637fe646bade2cc5ce50b745712292aa9c445b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/171830
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
As a small step toward speeding up timers, restrict modification
of the timer.when field to the timer code itself. Other code that
wants to change the when field of an existing timer must now call
resettimer rather than changing the when field and calling addtimer.
The new resettimer function also works for a new timer.
This is just a refactoring in preparation for later code.
Updates #27707
Change-Id: Iccd5dcad415ffbeac4c2a3cf015e91f82692acf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/171825
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
The new netpollBreak function can be used to interrupt a blocking netpoll.
This function is not currently used; it will be used by later CLs.
Updates #27707
Change-Id: I5cb936609ba13c3c127ea1368a49194fc58c9f4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/171824
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
You were a useful port and you've served your purpose.
Thanks for all the play.
A subsequent CL will remove amd64p32 (including assembly files and
toolchain bits) and remaining bits. The amd64p32 removal will be
separated into its own CL in case we want to support the Linux x32 ABI
in the future and want our old amd64p32 support as a starting point.
Updates #30439
Change-Id: Ia3a0c7d49804adc87bf52a4dea7e3d3007f2b1cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/199499
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Like GOOS=android which implies the "linux" build tag, GOOS=illumos
implies the "solaris" build tag. This lets the existing ecosystem of
packages still work on illumos, but still permits packages to start
differentiating between solaris and illumos.
Fixes #20603
Change-Id: I8f4eabf1a66060538dca15d7658c1fbc6c826622
Reviewed-on: https://go-review.googlesource.com/c/go/+/174457
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
This change makes it possible the runtime-integrated network poller and
APIs in the package internal/poll to report an event scanning error on a
read event.
The latest Go releases open up the way of the manipulation of the poller
for users. On the other hand, it starts misleading users into believing
that the poller accepts any user-configured file or socket perfectly
because of not reporting any error on event scanning, as mentioned in
issue 30426. The initial implementation of the poller was designed for
just well-configured, validated sockets produced by the package net.
However, the assumption is now obsolete.
Fixes #30624.
Benchmark results on linux/amd64:
benchmark old ns/op new ns/op delta
BenchmarkTCP4OneShot-4 24649 23979 -2.72%
BenchmarkTCP4OneShotTimeout-4 25742 24411 -5.17%
BenchmarkTCP4Persistent-4 5139 5222 +1.62%
BenchmarkTCP4PersistentTimeout-4 4919 4892 -0.55%
BenchmarkTCP6OneShot-4 21182 20767 -1.96%
BenchmarkTCP6OneShotTimeout-4 23364 22305 -4.53%
BenchmarkTCP6Persistent-4 4351 4366 +0.34%
BenchmarkTCP6PersistentTimeout-4 4227 4255 +0.66%
BenchmarkTCP4ConcurrentReadWrite-4 2309 1839 -20.36%
BenchmarkTCP6ConcurrentReadWrite-4 2180 1791 -17.84%
benchmark old allocs new allocs delta
BenchmarkTCP4OneShot-4 26 26 +0.00%
BenchmarkTCP4OneShotTimeout-4 26 26 +0.00%
BenchmarkTCP4Persistent-4 0 0 +0.00%
BenchmarkTCP4PersistentTimeout-4 0 0 +0.00%
BenchmarkTCP6OneShot-4 26 26 +0.00%
BenchmarkTCP6OneShotTimeout-4 26 26 +0.00%
BenchmarkTCP6Persistent-4 0 0 +0.00%
BenchmarkTCP6PersistentTimeout-4 0 0 +0.00%
BenchmarkTCP4ConcurrentReadWrite-4 0 0 +0.00%
BenchmarkTCP6ConcurrentReadWrite-4 0 0 +0.00%
benchmark old bytes new bytes delta
BenchmarkTCP4OneShot-4 2000 2000 +0.00%
BenchmarkTCP4OneShotTimeout-4 2000 2000 +0.00%
BenchmarkTCP4Persistent-4 0 0 +0.00%
BenchmarkTCP4PersistentTimeout-4 0 0 +0.00%
BenchmarkTCP6OneShot-4 2144 2144 +0.00%
BenchmarkTCP6OneShotTimeout-4 2144 2145 +0.05%
BenchmarkTCP6Persistent-4 0 0 +0.00%
BenchmarkTCP6PersistentTimeout-4 0 0 +0.00%
BenchmarkTCP4ConcurrentReadWrite-4 0 0 +0.00%
BenchmarkTCP6ConcurrentReadWrite-4 0 0 +0.00%
Change-Id: Iab60e504dff5639e688dc5420d852f336508c0af
Reviewed-on: https://go-review.googlesource.com/c/go/+/166497
Run-TryBot: Mikio Hara <mikioh.public.networking@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Created with:
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true iff" | grep -v vendor)
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true if" | grep -v vendor)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
This commit changes poll.PollDescriptor by poll.IsPollDescriptor. This
is needed for OS like AIX which have more than one FD using inside their
netpoll implementation.
Change-Id: I49e12a8d74045c501e19fdd8527cf166a3c64850
Reviewed-on: https://go-review.googlesource.com/c/146938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
We only need the memory barrier from these stores,
and we only store nil over nil or over a static function value.
The write barrier is unnecessary.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.0µs ± 0% -0.43% (p=0.032 n=5+5)
SetReadDeadline-6 205ns ± 1% 205ns ± 1% ~ (p=0.683 n=5+5)
Update #25729
Change-Id: I66c097a1db7188697ddfc381f31acec053dfed2c
Reviewed-on: https://go-review.googlesource.com/c/146345
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
runtimeNano is slower than nanotime, so pass the duration
to runtime_pollSetDeadline as is. netpoll can add nanotime itself.
Arguably a bit simpler because, say, a negative duration
clearly represents already expired timer, no need to compare to
nanotime again.
This may also fix an obscure corner case when a deadline in past
which happens to be nanotime 0 is confused with no deadline at all,
which are radically different things.
Also don't compute any durations and times if Time is zero
(currently we first compute everything and then reset d back to 0,
which is wasteful).
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.1µs ± 0% 17.0µs ± 0% ~ (p=0.421 n=5+5)
SetReadDeadline-6 230ns ± 0% 205ns ± 1% -10.63% (p=0.008 n=5+5)
Change-Id: I2aad699270289a5b9ead68f5e44ec4ec6d96baa0
Reviewed-on: https://go-review.googlesource.com/c/146344
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
|
|
We only need the memory barrier in poll_runtime_pollSetDeadline only
when one of the timers has fired, which is not the expected case.
Memory barrier can be somewhat expensive on some archs,
so execute it only if one of the timers has in fact fired.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.1µs ± 0% +0.35% (p=0.032 n=5+5)
SetReadDeadline-6 232ns ± 0% 230ns ± 0% -1.03% (p=0.000 n=4+5)
Update #25729
Change-Id: Ifce6f505b9e7ba3717bad8f454077a2e94ea6e75
Reviewed-on: https://go-review.googlesource.com/c/146343
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Currently when netpoll deadline is incrementally prolonged,
we delete and re-add timer each time.
Add modtimer function that does both and use it when we need
to modify an existing netpoll timer to avoid unnecessary lock/unlock.
TCP4OneShotTimeout-6 17.2µs ± 0% 17.0µs ± 0% -0.82% (p=0.008 n=5+5)
SetReadDeadline-6 274ns ± 2% 261ns ± 0% -4.89% (p=0.008 n=5+5)
Update #25729
Change-Id: I08b89dbbc1785dd180e967a37b0aa23b0c4613a8
Reviewed-on: https://go-review.googlesource.com/c/146339
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Currently we always delete both read and write timers and then
add them again. However, if user setups read and write deadline
separately, then we don't need to touch the other one.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.2µs ± 0% 17.2µs ± 0% ~ (p=0.310 n=5+5)
SetReadDeadline-6 319ns ± 1% 274ns ± 2% -13.94% (p=0.008 n=5+5)
Update #25729
Change-Id: I4c869c3083521de6d0cd6ca99a7609d4dd84b4e4
Reviewed-on: https://go-review.googlesource.com/c/146338
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
