[Git][ghc/ghc][wip/T18914] 89 commits: SMP.h: Add C11-style atomic operations

Ryan Scott gitlab at gitlab.haskell.org
Mon Nov 9 00:13:48 UTC 2020



Ryan Scott pushed to branch wip/T18914 at Glasgow Haskell Compiler / GHC


Commits:
b9d4dd9c by Ben Gamari at 2020-10-24T20:44:17-04:00
SMP.h: Add C11-style atomic operations

- - - - -
ccf2d4b0 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts: Infrastructure for testing with ThreadSanitizer

- - - - -
a61f66d6 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/CNF: Initialize all bdescrs in group

It seems wise and cheap to ensure that the whole bdescr of all blocks of
a compact group is valid, even if most cases only look at the flags
field.

- - - - -
65136c13 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/Capability: Intialize interrupt field

Previously this was left uninitialized.

Also clarify some comments.

- - - - -
b3ce6aca by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/Task: Make comments proper Notes

- - - - -
d3890ac7 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/SpinLock: Move to proper atomics

This is fairly straightforward; we just needed to use relaxed operations
for the PROF_SPIN counters and a release store instead of a write
barrier.

- - - - -
ef88712f by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/OSThreads: Fix data race

Previously we would race on the cached processor count. Avoiding this is
straightforward; just use relaxed operations.

- - - - -
33a719c3 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts/ClosureMaros: Use relaxed atomics

- - - - -
f08951fd by Ben Gamari at 2020-10-24T20:59:39-04:00
configure: Bump minimum-supported gcc version to 4.7

Since the __atomic_* builtins are not supported until gcc 4.7. Given
that this version was released in 2012 I think this is acceptable.

- - - - -
d584923a by Ben Gamari at 2020-10-24T20:59:39-04:00
testsuite: Fix thread leak in hs_try_putmvar00[13]

- - - - -
bf1b0bc7 by Ben Gamari at 2020-10-24T20:59:39-04:00
rts: Introduce SET_HDR_RELEASE

Also ensure that we also store the info table pointer last to ensure
that the synchronization covers all stores.

- - - - -
1a2e9f5e by Ben Gamari at 2020-10-24T21:00:19-04:00
gitlab-ci: Add nightly-x86_64-linux-deb9-tsan job

- - - - -
58a5b0e5 by GHC GitLab CI at 2020-10-24T21:00:19-04:00
testsuite: Mark setnumcapabilities001 as broken with TSAN

Due to #18808.

- - - - -
d9bc7dea by GHC GitLab CI at 2020-10-24T21:00:19-04:00
testsuite: Skip divbyzero and derefnull under TSAN

ThreadSanitizer changes the output of these tests.

- - - - -
fcc42a10 by Ben Gamari at 2020-10-24T21:00:19-04:00
testsuite: Skip high memory usage tests with TSAN

ThreadSanitizer significantly increases the memory footprint of tests,
so much so that it can send machines into OOM.

- - - - -
cae4bb3e by Ben Gamari at 2020-10-24T21:00:19-04:00
testsuite: Mark hie002 as high_memory_usage

This test has a peak residency of 1GByte; this is large enough to
classify as "high" in my book.

- - - - -
dae1b86a by Ben Gamari at 2020-10-24T21:00:19-04:00
testsuite: Mark T9872[abc] as high_memory_usage

These all have a maximum residency of over 2 GB.

- - - - -
c5a0bb22 by Ben Gamari at 2020-10-24T21:00:19-04:00
gitlab-ci: Disable documentation in TSAN build

Haddock chews through enough memory to cause the CI builders to OOM and
there's frankly no reason to build documentation in this job anyways.

- - - - -
4cb1232e by Ben Gamari at 2020-10-24T21:00:19-04:00
TSANUtils: Ensure that C11 atomics are supported

- - - - -
7ed15f7f by Ben Gamari at 2020-10-24T21:00:19-04:00
testsuite: Mark T3807 as broken with TSAN

Due to #18883.

- - - - -
f7e6f012 by Ben Gamari at 2020-10-24T21:00:19-04:00
testsuite: Mark T13702 as broken with TSAN due to #18884

- - - - -
16b136b0 by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Factor out logic to identify a good capability for running a task

Not only does this make the control flow a bit clearer but it also
allows us to add a TSAN suppression on this logic, which requires
(harmless) data races.

- - - - -
2781d68c by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Annotate benign race in waitForCapability

- - - - -
f6b4b492 by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Clarify locking behavior of releaseCapability_

- - - - -
65219810 by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Add assertions for task ownership of capabilities

- - - - -
31fa87ec by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Use relaxed atomics on n_returning_tasks

This mitigates the warning of a benign race on n_returning_tasks in
shouldYieldCapability.

See #17261.

- - - - -
6517a2ea by Ben Gamari at 2020-10-24T21:00:36-04:00
rts: Mitigate races in capability interruption logic

- - - - -
2e9ba3f2 by Ben Gamari at 2020-10-24T21:00:36-04:00
rts/Capability: Use relaxed operations for last_free_capability

- - - - -
e10dde37 by Ben Gamari at 2020-10-24T21:00:37-04:00
rts: Use relaxed operations for cap->running_task (TODO)

This shouldn't be necessary since only the owning thread of the capability
should be touching this.

- - - - -
855325cd by Ben Gamari at 2020-10-24T21:00:37-04:00
rts/Schedule: Use relaxed operations for sched_state

- - - - -
811f915d by Ben Gamari at 2020-10-24T21:00:37-04:00
rts: Accept data race in work-stealing implementation

This race is okay since the task is owned by the capability pushing it.
By Note [Ownership of Task] this means that the capability is free to
write to `task->cap` without taking `task->lock`.

Fixes #17276.

- - - - -
8d2b3c3d by Ben Gamari at 2020-10-24T21:00:37-04:00
rts: Eliminate data races on pending_sync

- - - - -
f8871018 by Ben Gamari at 2020-10-24T21:00:37-04:00
rts/Schedule: Eliminate data races on recent_activity

We cannot safely use relaxed atomics here.

- - - - -
d079b943 by Ben Gamari at 2020-10-24T21:00:37-04:00
rts: Avoid data races in message handling

- - - - -
06f80497 by Ben Gamari at 2020-10-24T21:00:37-04:00
rts/Messages: Drop incredibly fishy write barrier

executeMessage previously had a write barrier at the beginning of its
loop apparently in an attempt to synchronize with another thread's
writes to the Message. I would guess that the author had intended to use
a load barrier here given that there are no globally-visible writes done
in executeMessage.

I've removed the redundant barrier since the necessary load barrier is
now provided by the ACQUIRE_LOAD.

- - - - -
d4a87779 by Ben Gamari at 2020-10-24T21:00:38-04:00
rts/ThreadPaused: Avoid data races

- - - - -
56778ab3 by Ben Gamari at 2020-10-24T21:00:38-04:00
rts/Schedule: Eliminate data races in run queue management

- - - - -
086521f7 by Ben Gamari at 2020-10-24T21:00:38-04:00
rts: Eliminate shutdown data race on task counters

- - - - -
abad9778 by Ben Gamari at 2020-10-24T21:00:38-04:00
rts/Threads: Avoid data races (TODO)

Replace barriers with appropriate ordering. Drop redundant barrier in
tryWakeupThread (the RELEASE barrier will be provided by sendMessage's
mutex release).

We use relaxed operations on why_blocked and the stack although it's not
clear to me why this is necessary.

- - - - -
2f56be8a by Ben Gamari at 2020-10-24T21:00:39-04:00
rts/Messages: Annotate benign race

- - - - -
7c0cdab1 by Ben Gamari at 2020-10-24T21:00:39-04:00
rts/RaiseAsync: Synchronize what_next read

- - - - -
6cc2a8a5 by Ben Gamari at 2020-10-24T21:00:39-04:00
rts/Task: Move debugTrace to avoid data race

Specifically, we need to hold all_tasks_mutex to read taskCount.

- - - - -
bbaec97d by Ben Gamari at 2020-10-24T21:00:39-04:00
Disable flawed assertion

- - - - -
dd175a92 by Ben Gamari at 2020-10-24T21:00:39-04:00
Document schedulePushWork race

- - - - -
3416244b by Ben Gamari at 2020-10-24T21:00:40-04:00
Capabiliity: Properly fix data race on n_returning_tasks

There is a real data race but can be made safe by using proper atomic
(but relaxed) accesses.

- - - - -
dffd9432 by Ben Gamari at 2020-10-24T21:00:40-04:00
rts: Make write of to_cap->inbox atomic

This is necessary since emptyInbox may read from to_cap->inbox without
taking cap->lock.

- - - - -
1f4cbc29 by Ben Gamari at 2020-10-24T21:00:57-04:00
rts/BlockAlloc: Use relaxed operations

- - - - -
d0d07cff by Ben Gamari at 2020-10-24T21:00:57-04:00
rts: Rework handling of mutlist scavenging statistics

- - - - -
9e5c7f6d by Ben Gamari at 2020-10-24T21:00:57-04:00
rts: Avoid data races in StablePtr implementation

This fixes two potentially problematic data races in the StablePtr
implementation:

 * We would fail to RELEASE the stable pointer table when enlarging it,
   causing other cores to potentially see uninitialized memory.

 * We would fail to ACQUIRE when dereferencing a stable pointer.

- - - - -
316add67 by Ben Gamari at 2020-10-24T21:00:57-04:00
rts/Storage: Use atomics

- - - - -
5c23bc4c by Ben Gamari at 2020-10-24T21:00:58-04:00
rts/Updates: Use proper atomic operations

- - - - -
3d0f033c by Ben Gamari at 2020-10-24T21:00:58-04:00
rts/Weak: Eliminate data races

By taking all_tasks_mutex in stat_exit. Also better-document the fact
that the task statistics are protected by all_tasks_mutex.

- - - - -
edb4b92b by Ben Gamari at 2020-10-24T21:01:18-04:00
rts/WSDeque: Rewrite with proper atomics

After a few attempts at shoring up the previous implementation, I ended
up turning to the literature and now use the proven implementation,

> N.M. LĂȘ, A. Pop, A.Cohen, and F.Z. Nardelli. "Correct and Efficient
> Work-Stealing for Weak Memory Models". PPoPP'13, February 2013,
> ACM 978-1-4503-1922/13/02.

Note only is this approach formally proven correct under C11 semantics
but it is also proved to be a bit faster in practice.

- - - - -
d39bbd3d by Ben Gamari at 2020-10-24T21:01:33-04:00
rts: Use relaxed atomics for whitehole spin stats

- - - - -
8f802f38 by Ben Gamari at 2020-10-24T21:01:33-04:00
rts: Avoid lock order inversion during fork

Fixes #17275.

- - - - -
cef667b0 by GHC GitLab CI at 2020-10-24T21:01:34-04:00
rts: Use proper relaxe operations in getCurrentThreadCPUTime

Here we are doing lazy initialization; it's okay if we do the check more
than once, hence relaxed operation is fine.

- - - - -
8cf50eb1 by Ben Gamari at 2020-10-24T21:01:54-04:00
rts/STM: Use atomics

This fixes a potentially harmful race where we failed to synchronize
before looking at a TVar's current_value.

Also did a bit of refactoring to avoid abstract over management of
max_commits.

- - - - -
88a7ce38 by Ben Gamari at 2020-10-24T21:01:54-04:00
rts/stm: Strengthen orderings to SEQ_CST instead of volatile

Previously the `current_value`, `first_watch_queue_entry`, and
`num_updates` fields of `StgTVar` were marked as `volatile` in an
attempt to provide strong ordering. Of course, this isn't sufficient.

We now use proper atomic operations. In most of these cases I strengthen
the ordering all the way to SEQ_CST although it's possible that some
could be weakened with some thought.

- - - - -
f97c59ce by Ben Gamari at 2020-10-24T21:02:11-04:00
Mitigate data races in event manager startup/shutdown

- - - - -
c7c3f8aa by Ben Gamari at 2020-10-24T21:02:22-04:00
rts: Accept benign races in Proftimer

- - - - -
5a98dfca by Ben Gamari at 2020-10-24T21:02:22-04:00
rts: Pause timer while changing capability count

This avoids #17289.

- - - - -
01d95525 by Ben Gamari at 2020-10-24T21:02:22-04:00
Fix #17289

- - - - -
9a528985 by Ben Gamari at 2020-10-24T21:02:23-04:00
suppress #17289 (ticker) race

- - - - -
1726ec41 by Ben Gamari at 2020-10-24T21:02:23-04:00
rts: Fix timer initialization

Previously `initScheduler` would attempt to pause the ticker and in so
doing acquire the ticker mutex. However, initTicker, which is
responsible for initializing said mutex, hadn't been called
yet.

- - - - -
bfbe4366 by Ben Gamari at 2020-10-24T21:02:23-04:00
rts: Fix races in Pthread timer backend shudown

We can generally be pretty relaxed in the barriers here since the timer
thread is a loop.

- - - - -
297acc71 by Ben Gamari at 2020-10-24T21:02:44-04:00
rts/Stats: Hide a few unused unnecessarily global functions

- - - - -
aad1f803 by Ben Gamari at 2020-10-30T00:41:14-04:00
rts/GC: Use atomics

- - - - -
d0bc0517 by Ben Gamari at 2020-10-30T00:41:14-04:00
rts: Use RELEASE ordering in unlockClosure

- - - - -
d44f5232 by Ben Gamari at 2020-10-30T00:41:14-04:00
rts/Storage: Accept races on heap size counters

- - - - -
4e4a7386 by Ben Gamari at 2020-10-30T00:41:14-04:00
rts: Join to concurrent mark thread during shutdown

Previously we would take all capabilities but fail to join on the thread
itself, potentially resulting in a leaked thread.

- - - - -
a80cc857 by GHC GitLab CI at 2020-10-30T00:41:14-04:00
rts: Fix race in GC CPU time accounting

Ensure that the GC leader synchronizes with workers before calling
stat_endGC.

- - - - -
105d43db by Ben Gamari at 2020-10-30T14:02:19-04:00
rts/SpinLock: Separate out slow path

Not only is this in general a good idea, but it turns out that GCC
unrolls the retry loop, resulting is massive code bloat in critical
parts of the RTS (e.g. `evacuate`).

- - - - -
f7b45cde by Ben Gamari at 2020-10-30T14:02:19-04:00
rts: Use relaxed ordering on spinlock counters

- - - - -
b1dda153 by Ben Gamari at 2020-11-01T12:58:36-05:00
rts/Stats: Protect with mutex

While on face value this seems a bit heavy, I think it's far better than
enforcing ordering on every access.

- - - - -
5c2e6bce by Ben Gamari at 2020-11-01T12:58:36-05:00
rts: Tear down stats_mutex after exitHeapProfiling

Since the latter wants to call getRTSStats.

- - - - -
ef25aaa1 by Ben Gamari at 2020-11-01T13:02:11-05:00
rts: Annotate hopefully "benign" races in freeGroup

- - - - -
3a181553 by Ben Gamari at 2020-11-01T13:02:18-05:00
Strengthen ordering in releaseGCThreads

- - - - -
af474f62 by Ben Gamari at 2020-11-01T13:05:38-05:00
Suppress data race due to close

This suppresses the other side of a race during shutdown.

- - - - -
b4686bff by Ben Gamari at 2020-11-01T13:09:59-05:00
Merge branch 'wip/tsan/ci' into wip/tsan/all

- - - - -
b8e66e0e by Ben Gamari at 2020-11-01T13:10:01-05:00
Merge branch 'wip/tsan/storage' into wip/tsan/all

- - - - -
375512cf by Ben Gamari at 2020-11-01T13:10:02-05:00
Merge branch 'wip/tsan/wsdeque' into wip/tsan/all

- - - - -
65ebf07e by Ben Gamari at 2020-11-01T13:10:03-05:00
Merge branch 'wip/tsan/misc' into wip/tsan/all

- - - - -
55c375d0 by Ben Gamari at 2020-11-01T13:10:04-05:00
Merge branch 'wip/tsan/stm' into wip/tsan/all

- - - - -
a9f75fe2 by Ben Gamari at 2020-11-01T13:10:06-05:00
Merge branch 'wip/tsan/event-mgr' into wip/tsan/all

- - - - -
8325d658 by Ben Gamari at 2020-11-01T13:10:24-05:00
Merge branch 'wip/tsan/timer' into wip/tsan/all

- - - - -
07e82ba5 by Ben Gamari at 2020-11-01T13:10:35-05:00
Merge branch 'wip/tsan/stats' into wip/tsan/all

- - - - -
638f38c5 by Ben Gamari at 2020-11-08T09:29:16-05:00
Merge remote-tracking branch 'origin/wip/tsan/all'

- - - - -
22888798 by Ben Gamari at 2020-11-08T12:08:40-05:00
Fix haddock submodule

The previous merge mistakenly reverted it.

- - - - -
638e08ac by Ryan Scott at 2020-11-08T19:12:06-05:00
Use HsOuterExplicit in instance sigs in deriving-generated code

Issue #18914 revealed that `GeneralizedNewtypeDeriving` would generate code
that mentions unbound type variables, which is dangerously fragile. The
problem (and fix) is described in the new `Wrinkle: Use HsOuterExplicit`
in `Note [GND and QuantifiedConstraints]`. The gist of it: make sure to
put the top-level `forall`s in `deriving`-generated instance signatures in an
`HsOuterExplicit` to ensure that they scope over the bodies of methods
correctly. A side effect of this process is that it will expand any type
synonyms in the instance signature, which will surface any `forall`s that
are hidden underneath type synonyms (such as in the test case for #18914).

While I was in town, I also performed some maintenance on `NewHsTypeX`, which
powers `GeneralizedNewtypeDeriving`:

* I renamed `NewHsTypeX` to `HsCoreTy`, which more accurately describes its
  intended purpose (#15706).
* To make sure that mistakes similar to #18914 do not occur later, I added an
  additional validity check when renaming `HsCoreTy`s that complains if an
  `HsCoreTy`s contains an out-of-scope type variable. See the new
  `Note [Renaming HsCoreTys]` in `GHC.Rename.HsType` for the details.

Fixes #15706. Fixes #18914. Bumps the `haddock` submodule.

- - - - -


30 changed files:

- .gitlab-ci.yml
- compiler/GHC/Hs/Type.hs
- compiler/GHC/Rename/HsType.hs
- compiler/GHC/Tc/Deriv/Generate.hs
- compiler/GHC/Tc/Gen/HsType.hs
- compiler/GHC/Tc/Gen/Sig.hs
- compiler/GHC/Tc/TyCl/Instance.hs
- configure.ac
- hadrian/hadrian.cabal
- hadrian/src/Flavour.hs
- hadrian/src/Settings.hs
- + hadrian/src/Settings/Flavours/ThreadSanitizer.hs
- includes/Rts.h
- includes/rts/OSThreads.h
- includes/rts/SpinLock.h
- includes/rts/StablePtr.h
- + includes/rts/TSANUtils.h
- includes/rts/storage/ClosureMacros.h
- includes/rts/storage/Closures.h
- includes/rts/storage/GC.h
- includes/stg/SMP.h
- libraries/base/GHC/Event/Control.hs
- + rts/.tsan-suppressions
- rts/Capability.c
- rts/Capability.h
- rts/Messages.c
- rts/Proftimer.c
- rts/RaiseAsync.c
- rts/RtsStartup.c
- rts/SMPClosureOps.h


The diff was not included because it is too large.


View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c5da55a2d057cf2a534d316b620c3670f4a0b771...638e08ac0cfa6e1f5336a2d2ba72d2826d858f54

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c5da55a2d057cf2a534d316b620c3670f4a0b771...638e08ac0cfa6e1f5336a2d2ba72d2826d858f54
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20201108/0616ee53/attachment-0001.html>


More information about the ghc-commits mailing list