<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/go-git.git/src/runtime, branch dev.unified</title>
<subtitle>github.com: golang/go
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/'/>
<entry>
<title>runtime: clean up panic and deadlock lock ranks</title>
<updated>2022-08-04T15:31:49+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-20T21:44:45+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=44ff9bff0cd02642c37cce0223d25dc57230c8d2'/>
<id>44ff9bff0cd02642c37cce0223d25dc57230c8d2</id>
<content type='text'>
I'm not entirely sure why these locks are currently ranked "deadlock &lt;
panic" since we drop panic before acquiring deadlock, and we actually
want deadlock to be below panic because panic is implicitly below
everything else and we want deadlock to be, too. My best guess is that
we had this edge because we intentionally acquire deadlock twice to
deadlock, and that causes the lock rank checking to panic on the
second acquire.

Fix this in a more sensible way by capturing that deadlock can be
acquired in a self-cycle and flipping the rank to "panic &lt; deadlock"
to express that deadlock needs to be under all other locks, just like
panic.

For #53789.

Change-Id: I8809e5d102ce473bd3ace0ba07bf2200ef60263f
Reviewed-on: https://go-review.googlesource.com/c/go/+/418719
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
I'm not entirely sure why these locks are currently ranked "deadlock &lt;
panic" since we drop panic before acquiring deadlock, and we actually
want deadlock to be below panic because panic is implicitly below
everything else and we want deadlock to be, too. My best guess is that
we had this edge because we intentionally acquire deadlock twice to
deadlock, and that causes the lock rank checking to panic on the
second acquire.

Fix this in a more sensible way by capturing that deadlock can be
acquired in a self-cycle and flipping the rank to "panic &lt; deadlock"
to express that deadlock needs to be under all other locks, just like
panic.

For #53789.

Change-Id: I8809e5d102ce473bd3ace0ba07bf2200ef60263f
Reviewed-on: https://go-review.googlesource.com/c/go/+/418719
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: make the lock rank DAG make more sense</title>
<updated>2022-08-04T15:31:48+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-20T20:17:51+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=f42dc0de74f83d39e5ca1af72fc5334c73bd41f9'/>
<id>f42dc0de74f83d39e5ca1af72fc5334c73bd41f9</id>
<content type='text'>
This groups, comments, and generally reorganizes the lock rank graph
description by subsystem. It also introduces several pseudo-nodes that
more cleanly describe the inherent layering of lock ranks by
subsystem.

I believe this doesn't actually change the graph, but haven't verified
this.

For #53789.

Change-Id: I72f332f5a23b8217c7dc1b21411631ad48cee4b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/418718
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This groups, comments, and generally reorganizes the lock rank graph
description by subsystem. It also introduces several pseudo-nodes that
more cleanly describe the inherent layering of lock ranks by
subsystem.

I believe this doesn't actually change the graph, but haven't verified
this.

For #53789.

Change-Id: I72f332f5a23b8217c7dc1b21411631ad48cee4b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/418718
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: add mayAcquire annotation for finlock</title>
<updated>2022-08-04T15:31:46+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-20T19:06:31+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=d29a0282e9b7340ba2ed3f506e66304e92580238'/>
<id>d29a0282e9b7340ba2ed3f506e66304e92580238</id>
<content type='text'>
We're missing lock edges to finlock that happen only rarely. Anything
that calls mallocgc can potentially trigger sweeping, which can
potentially queue a finalizer, which acquires finlock. While this can
happen on any malloc, it happens relatively rarely, so we simply
haven't seen some of the lock edges that could happen.

Add a mayAcquire annotation to mallocgc to capture the possibility of
acquiring finlock.

With this change, we add "fin" to the set of "malloc" locks. Several
of these edges were already there, but not quite all of them.

This was found by inspecting the rank graph for things that didn't
make sense.

For #53789.

Change-Id: Idc10ce6f250596b0c07ba07ac93f2198fb38c22b
Reviewed-on: https://go-review.googlesource.com/c/go/+/418717
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We're missing lock edges to finlock that happen only rarely. Anything
that calls mallocgc can potentially trigger sweeping, which can
potentially queue a finalizer, which acquires finlock. While this can
happen on any malloc, it happens relatively rarely, so we simply
haven't seen some of the lock edges that could happen.

Add a mayAcquire annotation to mallocgc to capture the possibility of
acquiring finlock.

With this change, we add "fin" to the set of "malloc" locks. Several
of these edges were already there, but not quite all of them.

This was found by inspecting the rank graph for things that didn't
make sense.

For #53789.

Change-Id: Idc10ce6f250596b0c07ba07ac93f2198fb38c22b
Reviewed-on: https://go-review.googlesource.com/c/go/+/418717
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: add missing trace lock edges</title>
<updated>2022-08-04T15:31:45+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-21T01:49:15+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=c5be4ed7df3b2ae8f9d0a5c85afa4cf49e22a56d'/>
<id>c5be4ed7df3b2ae8f9d0a5c85afa4cf49e22a56d</id>
<content type='text'>
We're missing lock edges to trace.lock that happen only rarely. Any
trace event can potentially fill up a trace buffer and acquire
trace.lock in order to flush the buffer, but this happens relatively
rarely, so we simply haven't seen some of these lock edges that could
happen.

With this change, we promote "fin, notifyList &lt; traceStackTab" to
"fin, notifyList &lt; trace" and now everything that emits trace events
with a P enters the tracer lock ranks via "trace", rather than some
things entering at "trace" and others at "traceStackTab".

This was found by inspecting the rank graph for things that didn't
make sense.

Ideally we would add a mayAcquire annotation that any trace event can
potentially acquire trace.lock, but there are actually cases that
violate this ranking right now. This is #53979. The chance of a lock
cycle is extremely low given the number of conditions that have to
happen simultaneously.

For #53789.

Change-Id: Ic65947d27dee88d2daf639b21b2c9d37552f0ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/418716
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We're missing lock edges to trace.lock that happen only rarely. Any
trace event can potentially fill up a trace buffer and acquire
trace.lock in order to flush the buffer, but this happens relatively
rarely, so we simply haven't seen some of these lock edges that could
happen.

With this change, we promote "fin, notifyList &lt; traceStackTab" to
"fin, notifyList &lt; trace" and now everything that emits trace events
with a P enters the tracer lock ranks via "trace", rather than some
things entering at "trace" and others at "traceStackTab".

This was found by inspecting the rank graph for things that didn't
make sense.

Ideally we would add a mayAcquire annotation that any trace event can
potentially acquire trace.lock, but there are actually cases that
violate this ranking right now. This is #53979. The chance of a lock
cycle is extremely low given the number of conditions that have to
happen simultaneously.

For #53789.

Change-Id: Ic65947d27dee88d2daf639b21b2c9d37552f0ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/418716
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: generate the lock ranking from a DAG description</title>
<updated>2022-08-04T15:31:44+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-19T21:31:52+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=2b8a9a484fbc91b7b0d21890e33b28a0b48e3a10'/>
<id>2b8a9a484fbc91b7b0d21890e33b28a0b48e3a10</id>
<content type='text'>
Currently, the runtime lock rank graph is maintained manually in a
large set of arrays that give the partial order and a manual
topological sort of this partial order. Any changes to the rank graph
are difficult to reason about and hard to review, as well as likely to
cause merge conflicts. Furthermore, because the partial order is
manually maintained, it's not actually transitively closed (though
it's close), meaning there are many cases where rank a can be acquired
before b and b before c, but a cannot be acquired before c. While this
isn't technically wrong, it's very strange in the context of lock
ordering.

Replace all of this with a much more compact, readable, and
maintainable description of the rank graph written in the internal/dag
graph language. We statically generate the runtime structures from
this description, which has the advantage that the parser doesn't have
to run during runtime initialization and the structures can live in
static data where they can be accessed from any point during runtime
init.

The current description was automatically generated from the existing
partial order, combined with a transitive reduction. This ensures it's
correct, but it could use some manual messaging to call out the
logical layers and add some structure.

We do lose the ad hoc string names of the lock ranks in this
translation, which could mostly be derived from the rank constant
names, but not always. I may bring those back but in a more uniform
way.

We no longer need the tests in lockrank_test.go because they were
checking that we manually maintained the structures correctly.

Fixes #53789.

Change-Id: I54451d561b22e61150aff7e9b8602ba9737e1b9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/418715
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Currently, the runtime lock rank graph is maintained manually in a
large set of arrays that give the partial order and a manual
topological sort of this partial order. Any changes to the rank graph
are difficult to reason about and hard to review, as well as likely to
cause merge conflicts. Furthermore, because the partial order is
manually maintained, it's not actually transitively closed (though
it's close), meaning there are many cases where rank a can be acquired
before b and b before c, but a cannot be acquired before c. While this
isn't technically wrong, it's very strange in the context of lock
ordering.

Replace all of this with a much more compact, readable, and
maintainable description of the rank graph written in the internal/dag
graph language. We statically generate the runtime structures from
this description, which has the advantage that the parser doesn't have
to run during runtime initialization and the structures can live in
static data where they can be accessed from any point during runtime
init.

The current description was automatically generated from the existing
partial order, combined with a transitive reduction. This ensures it's
correct, but it could use some manual messaging to call out the
logical layers and add some structure.

We do lose the ad hoc string names of the lock ranks in this
translation, which could mostly be derived from the rank constant
names, but not always. I may bring those back but in a more uniform
way.

We no longer need the tests in lockrank_test.go because they were
checking that we manually maintained the structures correctly.

Fixes #53789.

Change-Id: I54451d561b22e61150aff7e9b8602ba9737e1b9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/418715
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: delete unused lock ranks</title>
<updated>2022-08-04T15:31:43+00:00</updated>
<author>
<name>Austin Clements</name>
<email>austin@google.com</email>
</author>
<published>2022-07-19T20:15:03+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=ddfd6394084f534bac3966d3a1736c15c665bd3a'/>
<id>ddfd6394084f534bac3966d3a1736c15c665bd3a</id>
<content type='text'>
For #53789.

Change-Id: Ic7379afcfdcc47b541bac9b44b5bc6b43604fc0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418714
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
For #53789.

Change-Id: Ic7379afcfdcc47b541bac9b44b5bc6b43604fc0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418714
Reviewed-by: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Run-TryBot: Austin Clements &lt;austin@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: support riscv64 SV57 mode</title>
<updated>2022-08-04T05:33:40+00:00</updated>
<author>
<name>Dmitry Vyukov</name>
<email>dvyukov@google.com</email>
</author>
<published>2022-05-27T16:55:35+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=1e3c19f3fee12e5e2b7802a54908a4d4d03960da'/>
<id>1e3c19f3fee12e5e2b7802a54908a4d4d03960da</id>
<content type='text'>
riscv64 has SV57 mode when user-space VA is 56 bits.
Linux kernel recently got support for this mode and Go binaries started crashing as:

runtime: lfstack.push invalid packing: node=0xffffff5908a940 cnt=0x1
packed=0xffff5908a9400001 -&gt; node=0xffff5908a940

Adjust lfstack code to use only 8 top bits of pointers on riscv64.

For context see:
https://groups.google.com/g/syzkaller-bugs/c/lU0GQTZoNQQ/m/O_c3vmE3AAAJ

Update #54104

Change-Id: Ib5d3d6a79c0c6eddf11618d73fcc8bc1832a9c25
Reviewed-on: https://go-review.googlesource.com/c/go/+/409055
Reviewed-by: Joel Sing &lt;joel@sing.id.au&gt;
Reviewed-by: Meng Zhuo &lt;mzh@golangcn.org&gt;
Reviewed-by: Michael Knyszek &lt;mknyszek@google.com&gt;
Reviewed-by: Cherry Mui &lt;cherryyz@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
riscv64 has SV57 mode when user-space VA is 56 bits.
Linux kernel recently got support for this mode and Go binaries started crashing as:

runtime: lfstack.push invalid packing: node=0xffffff5908a940 cnt=0x1
packed=0xffff5908a9400001 -&gt; node=0xffff5908a940

Adjust lfstack code to use only 8 top bits of pointers on riscv64.

For context see:
https://groups.google.com/g/syzkaller-bugs/c/lU0GQTZoNQQ/m/O_c3vmE3AAAJ

Update #54104

Change-Id: Ib5d3d6a79c0c6eddf11618d73fcc8bc1832a9c25
Reviewed-on: https://go-review.googlesource.com/c/go/+/409055
Reviewed-by: Joel Sing &lt;joel@sing.id.au&gt;
Reviewed-by: Meng Zhuo &lt;mzh@golangcn.org&gt;
Reviewed-by: Michael Knyszek &lt;mknyszek@google.com&gt;
Reviewed-by: Cherry Mui &lt;cherryyz@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: trivial replacements of g in remaining files</title>
<updated>2022-08-02T18:52:33+00:00</updated>
<author>
<name>Michael Pratt</name>
<email>mpratt@google.com</email>
</author>
<published>2022-07-20T17:18:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=29b9a328d268d53833d2cc063d1d8b4bf6852675'/>
<id>29b9a328d268d53833d2cc063d1d8b4bf6852675</id>
<content type='text'>
Rename g variables to gp for consistency.

Change-Id: I09ecdc7e8439637bc0e32f9c5f96f515e6436362
Reviewed-on: https://go-review.googlesource.com/c/go/+/418591
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Rename g variables to gp for consistency.

Change-Id: I09ecdc7e8439637bc0e32f9c5f96f515e6436362
Reviewed-on: https://go-review.googlesource.com/c/go/+/418591
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: trivial replacements of g in signal_unix.go</title>
<updated>2022-08-02T18:52:18+00:00</updated>
<author>
<name>Michael Pratt</name>
<email>mpratt@google.com</email>
</author>
<published>2022-07-20T17:18:42+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=c647264619bba886f1bff90e594d3cddc057f6ce'/>
<id>c647264619bba886f1bff90e594d3cddc057f6ce</id>
<content type='text'>
Change-Id: I0d8d50c8b7ae6cd01594091392dc726cf149e34a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418590
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Change-Id: I0d8d50c8b7ae6cd01594091392dc726cf149e34a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418590
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>runtime: tricky replacements of g in traceback.go</title>
<updated>2022-08-02T18:52:13+00:00</updated>
<author>
<name>Michael Pratt</name>
<email>mpratt@google.com</email>
</author>
<published>2022-07-20T17:18:54+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/go-git.git/commit/?id=399f50c9d773e17221e0c80ba1d739c5575c8460'/>
<id>399f50c9d773e17221e0c80ba1d739c5575c8460</id>
<content type='text'>
Change-Id: I80e847451b697de049a01df051a73b5976b39d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418589
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Change-Id: I80e847451b697de049a01df051a73b5976b39d4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/418589
Reviewed-by: Austin Clements &lt;austin@google.com&gt;
Run-TryBot: Michael Pratt &lt;mpratt@google.com&gt;
TryBot-Result: Gopher Robot &lt;gobot@golang.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
