Skip to contentSkip to conversation

rust: block: require `Sync` for `Operations::QueueData`

PATCH In review Andreas Hindborg 4 participants 5 messages Jun 8 – Jun 11

Subsystems (3)
Block layer Rust Block layer device driver API Rust
lore
Share & tools
lore

Andreas Hindborg

author maintainer
#0 lore raw

PATCH 1/1 rust: block: require `Sync` for `Operations::QueueData`

The queue data installed in a GenDisk is stored in the request queue and handed back to the driver as a shared borrow through the queue_rq and commit_rqs callbacks. Both callbacks obtain that borrow via ForeignOwnable::borrow and may execute concurrently on several CPUs, since the block layer runs one hardware queue per CPU. That means a shared reference to the same queue data can be live on multiple threads at once, which is only sound when the referent is Sync.

The initial GenDisk private data support omitted this bound, so a driver could install a non-Sync type as queue data and then access it concurrently from multiple CPUs without synchronization. Add a Sync bound to the QueueData associated type to rule that out.

Fixes 90d952fac8ac ("rust: block: add `GenDisk` private data support")
Signed-off-by Andreas Hindborg <[email protected]>
---
 rust/kernel/block/mq/operations.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
rust/kernel/block/mq/operations.rs +1 -1
index 8ad46129a52c..89029f468f44 100644
@@ -30,7 +30,7 @@
 pub trait Operations: Sized {
     /// Data associated with the `struct request_queue` that is allocated for
     /// the `GenDisk` associated with this `Operations` implementation.
-    type QueueData: ForeignOwnable;
+    type QueueData: ForeignOwnable + Sync;
 
     /// Called by the kernel to queue a request with the driver. If `is_last` is
     /// `false`, the driver is allowed to defer committing the request.

---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260608-queue-data-sync-80b66ab312ac

Best regards,

Signature · 1 line
Andreas Hindborg <[email protected]>
#1 lore raw
Quoted 2 lines · Andreas Hindborg
Fixes 90d952fac8ac ("rust: block: add `GenDisk` private data support")

Should this be Cc: stable given the hash is old? i.e. 6.18.y+

Cheers,
Miguel

author maintainer
#2 lore raw
Quoted 5 lines · Miguel Ojeda
Quoted 2 lines · Andreas Hindborg
Fixes 90d952fac8ac ("rust: block: add `GenDisk` private data support")

Should this be Cc: stable given the hash is old? i.e. 6.18.y+

Yes it should have cc stable, sorry.

Best regards,
Andreas Hindborg

reviewer Reviewed-by on v1 1/1
#3 lore raw
In reply to Andreas Hindborg
Quoted 8 lines · Andreas Hindborg

The queue data installed in a GenDisk is stored in the request queue and handed back to the driver as a shared borrow through the queue_rq and commit_rqs callbacks. Both callbacks obtain that borrow via ForeignOwnable::borrow and may execute concurrently on several CPUs, since the block layer runs one hardware queue per CPU. That means a shared reference to the same queue data can be live on multiple threads at once, which is only sound when the referent is Sync.

Quoted 7 lines

The initial GenDisk private data support omitted this bound, so a driver could install a non-Sync type as queue data and then access it concurrently from multiple CPUs without synchronization. Add a Sync bound to the QueueData associated type to rule that out.

Fixes 90d952fac8ac ("rust: block: add `GenDisk` private data support")
Signed-off-by Andreas Hindborg <[email protected]>
Reviewed-by Alice Ryhl <[email protected]>
reviewer Reviewed-by on v1 1/1
#4 lore raw
In reply to Andreas Hindborg
Quoted 8 lines · Andreas Hindborg

The queue data installed in a GenDisk is stored in the request queue and handed back to the driver as a shared borrow through the queue_rq and commit_rqs callbacks. Both callbacks obtain that borrow via ForeignOwnable::borrow and may execute concurrently on several CPUs, since the block layer runs one hardware queue per CPU. That means a shared reference to the same queue data can be live on multiple threads at once, which is only sound when the referent is Sync.

Quoted 7 lines

The initial GenDisk private data support omitted this bound, so a driver could install a non-Sync type as queue data and then access it concurrently from multiple CPUs without synchronization. Add a Sync bound to the QueueData associated type to rule that out.

Fixes 90d952fac8ac ("rust: block: add `GenDisk` private data support")
Signed-off-by Andreas Hindborg <[email protected]>
Reviewed-by Gary Guo <[email protected]>
Quoted 3 lines
---
 rust/kernel/block/mq/operations.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)