distinguish file backend thread names across backend instances #997
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has checked 338 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 253 | 1 | 84 | 0 |
Click to see the invalid file list
- lib/propolis/src/block/id.rs
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
A `FileBackend` spawns some number of threads to actually handle I/Os once it's eventually attached to a device that supports block operations. Those names were just `worker N`, though, and typically all file backends were created with the same number of workers, so a running Propolis would have `num-file-backend` threads named `worker 0`, `worker 1`, etc. This made finding which threads you care about somewhat more complicated than it needs to be. Instead, include a block backend ID, and plumb that to file backends to use in worker thread names. Now, for example: ``` $ pstack 28651 | grep 'worker 0' ------- thread# 21 / lwp# 21 [file backend 0/worker 0] ------- ------- thread# 29 / lwp# 29 [file backend 1/worker 0] ------- ------- thread# 45 / lwp# 45 [file backend 2/worker 0] ------- ------- thread# 49 / lwp# 49 [file backend 3/worker 0] ------- ------- thread# 53 / lwp# 53 [file backend 4/worker 0] ------- ------- thread# 57 / lwp# 57 [file backend 5/worker 0] ------- ------- thread# 61 / lwp# 61 [file backend 6/worker 0] ------- ------- thread# 65 / lwp# 65 [file backend 7/worker 0] ------- ------- thread# 69 / lwp# 69 [file backend 8/worker 0] ------- ------- thread# 73 / lwp# 73 [file backend 9/worker 0] ------- ------ thread# 77 / lwp# 77 [file backend 10/worker 0] ------- ``` yippee! yay! Having `type DeviceId = u32; type BackendId = u32` made me pretty suspicious we (I) would accidentally pass one where I meant the other and produce confusing probe output, so this comes with a new `define_id` macro to newtype the ID and provide a bit of glue to make it useful. This brings `hw::nvme::DeviceId` along for the change and gives a hopefully-more-obvious home for what these IDs are, are not, and how they relate - in particular, that while a VM consisting only of N NVMe disks, today, will have NVMe controllers 0..N with block devices 0..N and block backends 0..N, where each `i` is related to "the same thing", this is not at all reliable. Today, one could flip an NVMe disk to virtio-block and change NVMe->block mappings, while tomrorow we might make devices concurrently and be totally nondeterministic!
iximeow
force-pushed
the
ixi/distinct-file-worker-threads
branch
from
f2c047c to
fccb897
Compare
hawkw
approved these changes
Dec 23, 2025
Comment on lines
+17
to
+30
| //! ## Limitations | ||
| //! | ||
| //! A consumer of `propolis` is free to construct devices supporting block | ||
| //! backends in any order, and may happen to construct block backends in any | ||
| //! different arbitrary order. Attaching the two kinds of item together is also | ||
| //! up to the consumer of `propolis`, and there is no requirement that a | ||
| //! particular block backend must be connected to a particular device. | ||
| //! | ||
| //! Consequently, these identifiers are not stable for use in migration of a VM, | ||
| //! and must not be used in a way visible to a VM. They are unsuitable for | ||
| //! emulated device serial numbers, model numbers, etc. The destination | ||
| //! `propolis` may construct the same set of devices in a different order, | ||
| //! resulting in different run-time identifiers for a device at the same | ||
| //! location. |
Member
There was a problem hiding this comment.
do we provide any type of way for a consumer (such as a D script) to establish a mapping of these IDs to externally knowable objects? i see that the DTrace block_attach probe tells you the device ID and the backend ID. would we want to have a way to associate that with an emulated device that can be associated with a guest facing thing? or is that out of scope?
Member
Author
There was a problem hiding this comment.
block_attach is the only way so far, but I figure this is really part of a #482 -y future. that could include more general stats about an entity plus these kinds of IDs. the dirty secret is that as much as I'm warning against it here, we're really just testing on all-NVMe VMs and knowing that Propolis maps NVMe 0 to device attachment 0 to file backend 0, and that we create devices in the order listed in the spec.. :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A
FileBackendspawns some number of threads to actually handle I/Os once it's eventually attached to a device that supports block operations. Those names were justworker N, though, and typically all file backends were created with the same number of workers, so a running Propolis would havenum-file-backendthreads namedworker 0,worker 1, etc. This made finding which threads you care about somewhat more complicated than it needs to be.Instead, include a block backend ID, and plumb that to file backends to use in worker thread names. Now, for example:
yippee! yay!
Having
type DeviceId = u32; type BackendId = u32made me pretty suspicious we (I) would accidentally pass one where I meant the other and produce confusing probe output, so this comes with a newdefine_idmacro to newtype the ID and provide a bit of glue to make it useful.This brings
hw::nvme::DeviceIdalong for the change and gives a hopefully-more-obvious home for what these IDs are, are not, and how they relate - in particular, that while a VM consisting only of N NVMe disks, today, will have NVMe controllers 0..N with block devices 0..N and block backends 0..N, where eachiis related to "the same thing", this is not at all reliable. Today, one could flip an NVMe disk to virtio-block and change NVMe->block mappings, while tomrorow we might make devices concurrently and be totally nondeterministic!