feat(gdb): Multithreading support#1298
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1298 +/- ##
==========================================
+ Coverage 75.79% 76.58% +0.78%
==========================================
Files 27 29 +2
Lines 4033 4266 +233
==========================================
+ Hits 3057 3267 +210
- Misses 976 999 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4659754 to
c3cfe88
Compare
|
The commits before the The rest is still a bit WIP while I and @n0toose iron out the remaining bugs. |
|
Besides the mentioned issue (which is a fixable race-condition), this should now mostly work. |
|
And the failure that I think stems from the issue mentioned above is: |
|
Successful gdb invocation now look like this: |
|
There are still cases where this runs into but idk what causes those. |
|
uhyve-gdb-malformed-01.pdf |
|
uhyve-gdb-not-malformed-01.pdf |
|
ohno We might have to truncate thread ids to 31 bits instead of 32 bits... |
|
Looks good now. |
| } | ||
|
|
||
| impl Target for GdbUhyve { | ||
| impl Freewheel { |
There was a problem hiding this comment.
I'd prefer to move the Freewheel impls up to be directly after the struct declaration.
|
Unfortunately, a force-push was necessary to get rid of the merge-conflict. |
|
I think separating concerns would work better and would suggest that not expanding the original scope of the PR would work best. I currently have rather limited capacities; I can't predictably tell when I'd have the time to combine some of the changes with the tree that was worked on in parallel. Planning accordingly would be my advice. |
I'd be for squashing, e.g. for the typo fixes. |
|
Should I squash this PR and put the squashed version on this branch, or should I leave this as-is and let @jounathaen squash-merge this? |
|
Former, I believe. |
|
Please squash it. |
|
Okay, before I do that, I first try to merge this into the arch-indep branch version, in order to not have to untangle that later. |
Co-authored-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
1dc4a0e to
2a8c716
Compare
|
Okay, squashed all the |
This is a combination of 30 commits.
nit(gdb): remove single gdb restriction
feat(gdb): set affinity in threads, fetch vcpu id from object
nit(gdb): Improve cpu_affinity handling
nit(gdb): have swap use AcqRel ordering
fix(gdb): GDB truncates thread-ids supplied by us, so truncate them before supplying them to GDB to ensure consistency
```
[TRACE uhyvelib::linux::gdb] tid2vcpu = {140098616854208: 0}
[TRACE uhyvelib::linux] Active thread: 140098616854208
[TRACE uhyvelib::linux::gdb] tid_to_vcpuw(1078625984)
thread 'main' (4706) panicked at src/linux/gdb/mod.rs:237:37:
no entry found for key
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
```
>>> 140098616854208 & 1078625984
1078625984
>>> 140098616854208 ^ 1078625984
140097538228224
>>> x = 140098616854208 ^ 1078625984
>>> f"{x:#x}"
'0x7f6b00000000'
```
fix(gdb): Fix most cases of GDB not being able to interrupt the target
These were actually multiple issues in a trenchcoat:
- set_scheduler_lock is called very late (often just before resume)
- for cleanliness, we now always check if we're stopped (per vcpu), even on first iteration
- wait in the vcpu threads for the gdb connection to be fully initialized,
to ensure that the breakpoints are set correctly (`full_init_event`)
It appears that this leads to a breakpoint getting hit in _start,
but at least it otherwise appears to work perfectly almost always.
Remaining failures are probably due to the non-intended treatment of `full_init_event`
(it should've been paired with an `AtomicBool` to ensure spurious wakeups don't lead to protocol errors).
fix(gdb): Truncate TIDs to be provided to GDB to 31 bits
See also:
- daniel5151/gdbstub#193
- https://sourceware.org/bugzilla/show_bug.cgi?id=33979
nit(gdb): Rename 'struct Freewheel' to 'struct GdbVcpuManager'
nit(gdb): Rename 'fn backend_report_invalid_value' to 'fn backend_invalid_value'
nit(gdb): Rename Tid resolvers
nit(gdb): Fix VcpuWrapper::tid comment
nit(gdb): Rename 'ResumeMode::Freewheel' to 'ResumeMode::FreeWheeling'
nit(gdb/resume): renamse '.free_wheel' to '.r#continue'
nit(gdb): Rename '.finished_initializing' to '.set_finished_initializing'
chore(gdb): Mark GdbVcpuManager getters as non-pub
Co-authored-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
|
(sorry, had to fix the attribution in the second multithreading commit) (this is now ready to merge, afaik) |
|
Thank you both! |

This is an experiment.
Supersedes #1196.
Fixes #1088.