-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
mitigate ABA with 32-bit queue indices if possible #5042
Conversation
When 64-bit atomics are supported, use 32-bit queue indices. This greatly improves resilience to ABA and has no impact on performance on 64-bit platforms. Fixes: #5041
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this looks fine.
Please merge master into your branch to fix the CI issues. |
Nevermind, I could do it myself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
You don't need to add any more merge commits at this point. |
…rs#5042) When 64-bit atomics are supported, use 32-bit queue indices. This greatly improves resilience to ABA and has no impact on performance on 64-bit platforms. Fixes: tokio-rs#5041
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.21.2` -> `1.22.0` | | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.21.2` -> `1.22.0` | --- ### Release Notes <details> <summary>tokio-rs/tokio</summary> ### [`v1.22.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.22.0): Tokio v1.22.0 [Compare Source](tokio-rs/tokio@tokio-1.21.2...tokio-1.22.0) ##### Added - runtime: add `Handle::runtime_flavor` ([#​5138]) - sync: add `Mutex::blocking_lock_owned` ([#​5130]) - sync: add `Semaphore::MAX_PERMITS` ([#​5144]) - sync: add `merge()` to semaphore permits ([#​4948]) - sync: add `mpsc::WeakUnboundedSender` ([#​5189]) ##### Added (unstable) - process: add `Command::process_group` ([#​5114]) - runtime: export metrics about the blocking thread pool ([#​5161]) - task: add `task::id()` and `task::try_id()` ([#​5171]) ##### Fixed - macros: don't take ownership of futures in macros ([#​5087]) - runtime: fix Stacked Borrows violation in `LocalOwnedTasks` ([#​5099]) - runtime: mitigate ABA with 32-bit queue indices when possible ([#​5042]) - task: wake local tasks to the local queue when woken by the same thread ([#​5095]) - time: panic in release mode when `mark_pending` called illegally ([#​5093]) - runtime: fix typo in expect message ([#​5169]) - runtime: fix `unsync_load` on atomic types ([#​5175]) - task: elaborate safety comments in task deallocation ([#​5172]) - runtime: fix `LocalSet` drop in thread local ([#​5179]) - net: remove libc type leakage in a public API ([#​5191]) - runtime: update the alignment of `CachePadded` ([#​5106]) ##### Changed - io: make `tokio::io::copy` continue filling the buffer when writer stalls ([#​5066]) - runtime: remove `coop::budget` from `LocalSet::run_until` ([#​5155]) - sync: make `Notify` panic safe ([#​5154]) ##### Documented - io: fix doc for `write_i8` to use signed integers ([#​5040]) - net: fix doc typos for TCP and UDP `set_tos` methods ([#​5073]) - net: fix function name in `UdpSocket::recv` documentation ([#​5150]) - sync: typo in `TryLockError` for `RwLock::try_write` ([#​5160]) - task: document that spawned tasks execute immediately ([#​5117]) - time: document return type of `timeout` ([#​5118]) - time: document that `timeout` checks only before poll ([#​5126]) - sync: specify return type of `oneshot::Receiver` in docs ([#​5198]) ##### Internal changes - runtime: use const `Mutex::new` for globals ([#​5061]) - runtime: remove `Option` around `mio::Events` in io driver ([#​5078]) - runtime: remove a conditional compilation clause ([#​5104]) - runtime: remove a reference to internal time handle ([#​5107]) - runtime: misc time driver cleanup ([#​5120]) - runtime: move signal driver to runtime module ([#​5121]) - runtime: signal driver now uses I/O driver directly ([#​5125]) - runtime: start decoupling I/O driver and I/O handle ([#​5127]) - runtime: switch `io::handle` refs with scheduler:Handle ([#​5128]) - runtime: remove Arc from I/O driver ([#​5134]) - runtime: use signal driver handle via `scheduler::Handle` ([#​5135]) - runtime: move internal clock fns out of context ([#​5139]) - runtime: remove `runtime::context` module ([#​5140]) - runtime: keep driver cfgs in `driver.rs` ([#​5141]) - runtime: add `runtime::context` to unify thread-locals ([#​5143]) - runtime: rename some confusing internal variables/fns ([#​5151]) - runtime: move `coop` mod into `runtime` ([#​5152]) - runtime: move budget state to context thread-local ([#​5157]) - runtime: move park logic into runtime module ([#​5158]) - runtime: move `Runtime` into its own file ([#​5159]) - runtime: unify entering a runtime with `Handle::enter` ([#​5163]) - runtime: remove handle reference from each scheduler ([#​5166]) - runtime: move `enter` into `context` ([#​5167]) - runtime: combine context and entered thread-locals ([#​5168]) - runtime: fix accidental unsetting of current handle ([#​5178]) - runtime: move `CoreStage` methods to `Core` ([#​5182]) - sync: name mpsc semaphore types ([#​5146]) [#​4948]: tokio-rs/tokio#4948 [#​5040]: tokio-rs/tokio#5040 [#​5042]: tokio-rs/tokio#5042 [#​5061]: tokio-rs/tokio#5061 [#​5066]: tokio-rs/tokio#5066 [#​5073]: tokio-rs/tokio#5073 [#​5078]: tokio-rs/tokio#5078 [#​5087]: tokio-rs/tokio#5087 [#​5093]: tokio-rs/tokio#5093 [#​5095]: tokio-rs/tokio#5095 [#​5099]: tokio-rs/tokio#5099 [#​5104]: tokio-rs/tokio#5104 [#​5106]: tokio-rs/tokio#5106 [#​5107]: tokio-rs/tokio#5107 [#​5114]: tokio-rs/tokio#5114 [#​5117]: tokio-rs/tokio#5117 [#​5118]: tokio-rs/tokio#5118 [#​5120]: tokio-rs/tokio#5120 [#​5121]: tokio-rs/tokio#5121 [#​5125]: tokio-rs/tokio#5125 [#​5126]: tokio-rs/tokio#5126 [#​5127]: tokio-rs/tokio#5127 [#​5128]: tokio-rs/tokio#5128 [#​5130]: tokio-rs/tokio#5130 [#​5134]: tokio-rs/tokio#5134 [#​5135]: tokio-rs/tokio#5135 [#​5138]: tokio-rs/tokio#5138 [#​5138]: tokio-rs/tokio#5138 [#​5139]: tokio-rs/tokio#5139 [#​5140]: tokio-rs/tokio#5140 [#​5141]: tokio-rs/tokio#5141 [#​5143]: tokio-rs/tokio#5143 [#​5144]: tokio-rs/tokio#5144 [#​5144]: tokio-rs/tokio#5144 [#​5146]: tokio-rs/tokio#5146 [#​5150]: tokio-rs/tokio#5150 [#​5151]: tokio-rs/tokio#5151 [#​5152]: tokio-rs/tokio#5152 [#​5154]: tokio-rs/tokio#5154 [#​5155]: tokio-rs/tokio#5155 [#​5157]: tokio-rs/tokio#5157 [#​5158]: tokio-rs/tokio#5158 [#​5159]: tokio-rs/tokio#5159 [#​5160]: tokio-rs/tokio#5160 [#​5161]: tokio-rs/tokio#5161 [#​5163]: tokio-rs/tokio#5163 [#​5166]: tokio-rs/tokio#5166 [#​5167]: tokio-rs/tokio#5167 [#​5168]: tokio-rs/tokio#5168 [#​5169]: tokio-rs/tokio#5169 [#​5171]: tokio-rs/tokio#5171 [#​5172]: tokio-rs/tokio#5172 [#​5175]: tokio-rs/tokio#5175 [#​5178]: tokio-rs/tokio#5178 [#​5179]: tokio-rs/tokio#5179 [#​5182]: tokio-rs/tokio#5182 [#​5189]: tokio-rs/tokio#5189 [#​5191]: tokio-rs/tokio#5191 [#​5198]: tokio-rs/tokio#5198 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNy4xIiwidXBkYXRlZEluVmVyIjoiMzQuMjkuMiJ9--> Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1651 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>
When 64-bit atomics are supported, use 32-bit queue indices. This greatly improves resilience to ABA and has no impact on performance on 64-bit platforms.
Fixes: #5041
Motivation
Currently, a stealer preempted at the wrong moment could attempt to steal more tasks than are available in the queue, resulting in UB. This is due to a potential ABA where the stealer head wraps around due to many push/pop cycles, leading the stealer to wrongly conclude that the state of the queue has not changed.
Solution
The proposed solution is to strongly improve ABA resilience by using 32-bit instead of 16-bit buffer indices when possible.
At the moment, "when possible" is determined based on the availability of 64-bit atomics, as reported by the
cfg_has_atomic_u64
macro. Another solution would be to usecfg(target_pointer_width = "64")
to exclude 32-bit targets that support 64-bit atomics natively, such asarmv7
, since these targets may see an impact in performance.No performance regression was observed on local tests on a
x86_64
target using thert_multi_threaded
benchmark.