* [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs
@ 2026-08-12 8:51 Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
Add support for reserving of ranges of IDs, with a usage in nova-core
for channel IDs. This entails adding bindings for the C bitmap
API for ranges of bits, then users of that in `IdPool`, and finally a
user of `IdPool` in nova-core, `ChannelIdPool`.
Channel ID tracking is needed for allotting ranges of channel IDs to
vGPU guests, and later for regular host channel ID reservation.
nova-core needs allocation of a contiguous sequence of IDs with a
specific length and sometimes a specific alignment [1].
About the tradeoffs between different data structures:
- IDA/xarray do not support allocating a contiguous sequence of IDs
(ida_alloc_range() allocates a single ID within a range, not a contiguous
sequence).
- A maple tree works, but is not as good a fit. The ID space is small
(limited to 2048) and aligned allocation needs an alloc_range()+erase() retry
loop (plus a Mutex around it, or new mas_empty_area() bindings) that
essentially reimplements bitmap_find_next_zero_area(). See the maple tree
version at [2]. For 2048 IDs a bitmap is also considerably faster and smaller
[3].
- The bitmap API natively supports aligned contiguous area allocation
(bitmap_find_next_zero_area()).
This is based on drm-rust-next.
[1]: https://lore.kernel.org/all/84bc8bd2-e292-4b84-9580-a1b5df4c5bdc@nvidia.com/
[2]: https://lore.kernel.org/all/20260710-chid-maple-v1-1-4ee869055268@nvidia.com/
[3]: https://lore.kernel.org/all/20260717053241.916441-1-ynorov@nvidia.com/
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v5:
- `bitmap_assert!` i32::MAX length for Bitmap::from_raw* (Yury)
- Only run overflow check on 32-bit (Yury)
- Link to v4: https://patch.msgid.link/20260810-chid-v4-0-c9f206fdcb97@nvidia.com
Changes in v4:
- Add `next_zero_area_off` to match C code (Yury)
- Replace overflow checks to match C code in bitmap-for-next.
- Tighten `Bitmap` unsafe contract to disallow Bitmaps larger than i32::MAX
- Link to v3: https://patch.msgid.link/20260729-chid-v3-0-20cc08032bbc@nvidia.com
Changes in v3:
- Use `Alignment` type in id_pool and bitmap (Alice)
- Remove hang check on the basis that it's extraordinarily rare.
- Link to v2: https://patch.msgid.link/20260723-chid-v2-0-c35e5e9fb3d9@nvidia.com
Changes in v2:
- Collected Alice's Reviewed-by on patch 1.
- Address Yury's comments w.r.t. using __bitmap_set etc directly.
- Address Yury's comments w.r.t. following the C names
- Additionally check for an overflow case that causes a hang
- Added more info to cover letter + patch 4 w.r.t. channel ID allottment
requirements
- Add align parameter to ChannelIdPool::alloc_area() plus an aligned
allocation test
- Add missing INVARIANT comment when constructing UnusedArea
- Link to v1:
https://patch.msgid.link/20260703-chid-v1-0-84fe8259e46e@nvidia.com
---
Eliot Courtney (5):
rust: bitmap: use function-level cfg on kunit test
rust: bitmap: restrict bitmap length to at most i32::MAX
rust: bitmap: add contiguous area operations
rust: id_pool: add contiguous area allocation
gpu: nova-core: add ChannelIdPool
drivers/gpu/nova-core/gpu.rs | 2 +
drivers/gpu/nova-core/gpu/channel.rs | 180 +++++++++++++++++++
rust/kernel/bitmap.rs | 329 +++++++++++++++++++++++++++++++----
rust/kernel/id_pool.rs | 69 ++++++++
4 files changed, 545 insertions(+), 35 deletions(-)
---
base-commit: 4c9ba407018e8deb06dbc643112bac8f40404f95
change-id: 20260608-chid-18fa943c6d6c
Best regards,
--
Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
@ 2026-08-12 8:51 ` Eliot Courtney
2026-08-12 22:23 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
Since commit c652dc44192d ("rust: kunit: allow `cfg` on `test`s"),
we no longer need this workaround.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/bitmap.rs | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
index b27e0ec80d64..a43bfe0ec3dc 100644
--- a/rust/kernel/bitmap.rs
+++ b/rust/kernel/bitmap.rs
@@ -572,24 +572,21 @@ fn bitmap_set_clear_find() -> Result<(), AllocError> {
}
#[test]
+ #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
fn owned_bitmap_out_of_bounds() -> Result<(), AllocError> {
- // TODO: Kunit #[test]s do not support `cfg` yet,
- // so we add it here in the body.
- #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
- {
- let mut b = BitmapVec::new(128, GFP_KERNEL)?;
- b.set_bit(2048);
- b.set_bit_atomic(2048);
- b.clear_bit(2048);
- b.clear_bit_atomic(2048);
- assert_eq!(None, b.next_bit(2048));
- assert_eq!(None, b.next_zero_bit(2048));
- assert_eq!(None, b.last_bit());
- }
+ let mut b = BitmapVec::new(128, GFP_KERNEL)?;
+
+ b.set_bit(2048);
+ b.set_bit_atomic(2048);
+ b.clear_bit(2048);
+ b.clear_bit_atomic(2048);
+ assert_eq!(None, b.next_bit(2048));
+ assert_eq!(None, b.next_zero_bit(2048));
+ assert_eq!(None, b.last_bit());
Ok(())
}
- // TODO: uncomment once kunit supports [should_panic] and `cfg`.
+ // TODO: uncomment once kunit supports `#[should_panic]`.
// #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
// #[test]
// #[should_panic]
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
@ 2026-08-12 8:51 ` Eliot Courtney
2026-08-12 19:44 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 3/5] rust: bitmap: add contiguous area operations Eliot Courtney
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
It is currently possible to construct a non-`BitmapVec` backed
`Bitmap` using `Bitmap::from_raw` that is larger than `i32::MAX`, and
it is not part of the unsafe requirements. Restricting all bitmaps
(even non-`BitmapVec` backed ones) to a maximum size of `i32::MAX`
simplifies a few things and matches `BitmapVec::MAX_LEN`.
Add that requirement to the unsafe requirements on `Bitmap::from_raw`
and `Bitmap::from_raw_mut`, and to the invariants on `Bitmap`.
This also fixes u32 casts truncating in `copy_and_extend`, which could
otherwise lead to OOB writes.
Fixes: 11eca92a2cae ("rust: add bitmap API.")
Link: https://lore.kernel.org/DKG0U8RLO7LZ.2I1AIH0S38PAP@nvidia.com
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/bitmap.rs | 68 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 21 deletions(-)
diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
index a43bfe0ec3dc..fdcfc0409773 100644
--- a/rust/kernel/bitmap.rs
+++ b/rust/kernel/bitmap.rs
@@ -17,24 +17,57 @@
/// # Invariants
///
/// Must reference a `[c_ulong]` long enough to fit `data.len()` bits.
+/// Must not be longer than `i32::MAX` bits.
#[cfg_attr(CONFIG_64BIT, repr(align(8)))]
#[cfg_attr(not(CONFIG_64BIT), repr(align(4)))]
pub struct Bitmap {
data: [()],
}
+macro_rules! bitmap_assert {
+ ($cond:expr, $($arg:tt)+) => {
+ #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
+ assert!($cond, $($arg)*);
+ }
+}
+
+macro_rules! bitmap_assert_return {
+ ($cond:expr, $($arg:tt)+) => {
+ #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
+ assert!($cond, $($arg)*);
+
+ #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
+ if !($cond) {
+ pr_err!($($arg)*);
+ return
+ }
+ }
+}
+
impl Bitmap {
/// Borrows a C bitmap.
///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds `i32::MAX`.
+ ///
/// # Safety
///
/// * `ptr` holds a non-null address of an initialized array of `unsigned long`
/// that is large enough to hold `nbits` bits.
+ /// * `nbits` must not exceed `i32::MAX`.
/// * the array must not be freed for the lifetime of this [`Bitmap`]
/// * concurrent access only happens through atomic operations
pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a Bitmap {
+ bitmap_assert!(
+ nbits <= i32::MAX as usize,
+ "`nbits` must be <= {}, was {}",
+ i32::MAX,
+ nbits
+ );
let data: *const [()] = core::ptr::slice_from_raw_parts(ptr.cast(), nbits);
// INVARIANT: `data` references an initialized array that can hold `nbits` bits.
+ // INVARIANT: the caller guarantees that `nbits` does not exceed `i32::MAX`.
// SAFETY:
// The caller guarantees that `data` (derived from `ptr` and `nbits`)
// points to a valid, initialized, and appropriately sized memory region
@@ -51,15 +84,27 @@ pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a Bitmap {
/// Borrows a C bitmap exclusively.
///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds `i32::MAX`.
+ ///
/// # Safety
///
/// * `ptr` holds a non-null address of an initialized array of `unsigned long`
/// that is large enough to hold `nbits` bits.
+ /// * `nbits` must not exceed `i32::MAX`.
/// * the array must not be freed for the lifetime of this [`Bitmap`]
/// * no concurrent access may happen.
pub unsafe fn from_raw_mut<'a>(ptr: *mut usize, nbits: usize) -> &'a mut Bitmap {
+ bitmap_assert!(
+ nbits <= i32::MAX as usize,
+ "`nbits` must be <= {}, was {}",
+ i32::MAX,
+ nbits
+ );
let data: *mut [()] = core::ptr::slice_from_raw_parts_mut(ptr.cast(), nbits);
// INVARIANT: `data` references an initialized array that can hold `nbits` bits.
+ // INVARIANT: the caller guarantees that `nbits` does not exceed `i32::MAX`.
// SAFETY:
// The caller guarantees that `data` (derived from `ptr` and `nbits`)
// points to a valid, initialized, and appropriately sized memory region
@@ -96,26 +141,6 @@ union BitmapRepr {
ptr: NonNull<usize>,
}
-macro_rules! bitmap_assert {
- ($cond:expr, $($arg:tt)+) => {
- #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
- assert!($cond, $($arg)*);
- }
-}
-
-macro_rules! bitmap_assert_return {
- ($cond:expr, $($arg:tt)+) => {
- #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
- assert!($cond, $($arg)*);
-
- #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
- if !($cond) {
- pr_err!($($arg)*);
- return
- }
- }
-}
-
/// Represents an owned bitmap.
///
/// Wraps underlying C bitmap API. See [`Bitmap`] for available
@@ -415,7 +440,8 @@ pub fn clear_bit_atomic(&self, index: usize) {
#[inline]
pub fn copy_and_extend(&mut self, src: &Bitmap) {
let len = core::cmp::min(src.len(), self.len());
- // SAFETY: access to `self` and `src` is within bounds.
+ // SAFETY: access to `self` and `src` is within bounds. Both lengths fit in `u32`
+ // because a `Bitmap` is at most `i32::MAX` bits, so the casts are lossless.
unsafe {
bindings::bitmap_copy_and_extend(
self.as_mut_ptr(),
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 3/5] rust: bitmap: add contiguous area operations
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
@ 2026-08-12 8:51 ` Eliot Courtney
2026-08-12 20:31 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 4/5] rust: id_pool: add contiguous area allocation Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool Eliot Courtney
4 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
Add bindings for area operations on bitmaps. Each one is
made safe by adding some extra checks compared to the underlying C code
(for example, checking bounds) and with additional checks to catch
likely erroneous usage if `CONFIG_RUST_BITMAP_HARDENED` is on.
Add tests demonstrating the edge cases.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/bitmap.rs | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 236 insertions(+)
diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
index fdcfc0409773..74c92cc452c9 100644
--- a/rust/kernel/bitmap.rs
+++ b/rust/kernel/bitmap.rs
@@ -10,6 +10,7 @@
use crate::bindings;
#[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
use crate::pr_err;
+use crate::ptr::Alignment;
use core::ptr::NonNull;
/// Represents a C bitmap. Wraps underlying C bitmap API.
@@ -523,6 +524,139 @@ pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
Some(index)
}
}
+
+ /// Finds a contiguous area of `nbits` zero bits at or after `start`, where the area plus
+ /// `align_offset` is aligned to `align`.
+ ///
+ /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
+ /// the bitmap exists.
+ ///
+ /// The returned index plus `align_offset` is a multiple of `align`.
+ ///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
+ #[inline]
+ pub fn next_zero_area_off(
+ &self,
+ start: usize,
+ nbits: usize,
+ align: Alignment,
+ align_offset: usize,
+ ) -> Option<usize> {
+ bitmap_assert!(
+ start < self.len(),
+ "`start` must be < {}, was {}",
+ self.len(),
+ start
+ );
+
+ let nr = u32::try_from(nbits).ok()?;
+ let align_mask = align.as_usize() - 1;
+
+ // The C alignment and end arithmetic must not overflow, or it can read out of bounds.
+ // Overflow is only possible on 32-bit.
+ #[cfg(not(CONFIG_64BIT))]
+ align_mask.checked_add(self.len())?.checked_add(nbits)?;
+
+ // SAFETY: `bitmap_find_next_zero_area_off` is safe to use with an out of bounds `start`
+ // value and, given the overflow check above, never reads beyond `self.len()` bits.
+ let index = unsafe {
+ bindings::bitmap_find_next_zero_area_off(
+ self.as_ptr().cast_mut(),
+ self.len(),
+ start,
+ nr,
+ align_mask,
+ align_offset,
+ )
+ };
+
+ (index < self.len()).then_some(index)
+ }
+
+ /// Finds a contiguous area of `nbits` zero bits at or after `start`, aligned to `align`.
+ ///
+ /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
+ /// the bitmap exists.
+ ///
+ /// The returned index is a multiple of `align`.
+ ///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
+ /// use kernel::bitmap::BitmapVec;
+ /// use kernel::ptr::Alignment;
+ ///
+ /// let mut b = BitmapVec::new(64, GFP_KERNEL)?;
+ /// let unaligned = Alignment::new::<1>();
+ ///
+ /// assert_eq!(Some(0), b.next_zero_area(0, 8, unaligned));
+ /// b.set(0, 5);
+ /// assert_eq!(Some(5), b.next_zero_area(0, 8, unaligned));
+ /// assert_eq!(Some(8), b.next_zero_area(0, 8, Alignment::new::<8>()));
+ /// assert_eq!(None, b.next_zero_area(0, 65, unaligned));
+ /// # Ok::<(), AllocError>(())
+ /// ```
+ #[inline]
+ pub fn next_zero_area(&self, start: usize, nbits: usize, align: Alignment) -> Option<usize> {
+ self.next_zero_area_off(start, nbits, align, 0)
+ }
+
+ /// Sets a contiguous area of `nbits` bits starting at `start`.
+ ///
+ /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
+ /// bounds, does nothing.
+ ///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
+ /// of bounds.
+ #[inline]
+ pub fn set(&mut self, start: usize, nbits: usize) {
+ bitmap_assert_return!(
+ start
+ .checked_add(nbits)
+ .is_some_and(|end| end <= self.len()),
+ "Area `start..start + nbits` ({}..{}) must be within bounds {}",
+ start,
+ start.saturating_add(nbits),
+ self.len()
+ );
+ // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
+ // `i32::MAX` bits, so the casts are lossless.
+ unsafe { bindings::__bitmap_set(self.as_mut_ptr(), start as u32, nbits as i32) };
+ }
+
+ /// Clears a contiguous area of `nbits` bits starting at `start`.
+ ///
+ /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
+ /// bounds, does nothing.
+ ///
+ /// # Panics
+ ///
+ /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
+ /// of bounds.
+ #[inline]
+ pub fn clear(&mut self, start: usize, nbits: usize) {
+ bitmap_assert_return!(
+ start
+ .checked_add(nbits)
+ .is_some_and(|end| end <= self.len()),
+ "Area `start..start + nbits` ({}..{}) must be within bounds {}",
+ start,
+ start.saturating_add(nbits),
+ self.len()
+ );
+ // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
+ // `i32::MAX` bits, so the casts are lossless.
+ unsafe { bindings::__bitmap_clear(self.as_mut_ptr(), start as u32, nbits as i32) };
+ }
}
#[cfg(CONFIG_RUST_BITMAP_KUNIT_TEST)]
@@ -640,4 +774,106 @@ fn bitmap_copy_and_extend() -> Result<(), AllocError> {
assert_eq!(Some(17), long_bitmap.last_bit());
Ok(())
}
+
+ #[test]
+ fn bitmap_area_set_clear_find() -> Result<(), AllocError> {
+ let mut b = BitmapVec::new(128, GFP_KERNEL)?;
+ let unaligned = Alignment::new::<1>();
+
+ assert_eq!(Some(0), b.next_zero_area(0, 5, unaligned));
+ b.set(0, 5); // Now contains {[0, 5)}.
+
+ assert_eq!(Some(0), b.next_bit(0));
+ assert_eq!(Some(4), b.next_bit(4));
+ assert_eq!(Some(5), b.next_zero_bit(0));
+ assert_eq!(Some(5), b.next_zero_area(0, 5, unaligned));
+ assert_eq!(Some(8), b.next_zero_area(0, 5, Alignment::new::<8>()));
+
+ b.set(8, 8); // Now contains {[0, 5), [8, 16)}.
+ assert_eq!(Some(16), b.next_zero_area(0, 4, Alignment::new::<16>()));
+ assert_eq!(Some(16), b.next_zero_area(0, 4, unaligned));
+
+ b.clear(0, 5); // Now contains {[8, 16)}.
+ assert_eq!(Some(0), b.next_zero_area(0, 5, unaligned));
+ assert_eq!(Some(8), b.next_bit(0));
+ assert_eq!(Some(15), b.last_bit());
+
+ b.clear(16, 0); // Zero-length in-bounds clears are no-ops.
+ assert_eq!(Some(8), b.next_bit(0));
+ assert_eq!(Some(15), b.last_bit());
+
+ // A zero-length request returns the first aligned position at or
+ // after the next zero bit, even if that position's own bit is set.
+ assert_eq!(Some(1), b.next_zero_area(1, 0, unaligned));
+ assert_eq!(Some(8), b.next_zero_area(1, 0, Alignment::new::<8>()));
+
+ b.set(60, 10); // Now contains {[8, 16), [60, 70)}.
+ assert_eq!(Some(60), b.next_bit(16));
+ assert_eq!(Some(69), b.last_bit());
+ assert_eq!(Some(16), b.next_zero_area(9, 40, unaligned));
+ assert_eq!(Some(70), b.next_zero_area(0, 45, unaligned));
+
+ b.clear(62, 6); // Now contains {[8, 16), [60, 62), [68, 70)}.
+ assert_eq!(Some(62), b.next_zero_area(60, 6, unaligned));
+ assert_eq!(Some(61), b.next_bit(61));
+ assert_eq!(Some(69), b.last_bit());
+
+ b.set(64, 0); // Zero-length in-bounds sets are no-ops.
+ assert_eq!(Some(62), b.next_zero_bit(62));
+ Ok(())
+ }
+
+ #[test]
+ fn bitmap_area_exhaustion() -> Result<(), AllocError> {
+ let mut b = BitmapVec::new(64, GFP_KERNEL)?;
+ let unaligned = Alignment::new::<1>();
+
+ assert_eq!(None, b.next_zero_area(0, 65, unaligned));
+ assert_eq!(None, b.next_zero_area(0, usize::MAX, unaligned));
+ assert_eq!(None, b.next_zero_area(1, usize::MAX, unaligned));
+
+ b.set_bit(0); // Now contains {[0, 1)}.
+ assert_eq!(None, b.next_zero_area(0, usize::MAX, unaligned));
+
+ b.set(0, 61); // Now contains {[0, 61)}.
+ assert_eq!(None, b.next_zero_area(0, 4, unaligned));
+ assert_eq!(Some(61), b.next_zero_area(0, 3, unaligned));
+ assert_eq!(None, b.next_zero_area(0, 1, Alignment::new::<64>()));
+ Ok(())
+ }
+
+ #[test]
+ fn bitmap_area_off() -> Result<(), AllocError> {
+ let mut b = BitmapVec::new(64, GFP_KERNEL)?;
+ let align8 = Alignment::new::<8>();
+
+ b.set(0, 5); // Now contains {[0, 5)}.
+
+ // The area plus align_offset starts at a multiple of the alignment.
+ assert_eq!(Some(7), b.next_zero_area_off(0, 8, align8, 1));
+ assert_eq!(Some(5), b.next_zero_area_off(0, 8, align8, 3));
+
+ // A zero offset behaves like next_zero_area().
+ assert_eq!(
+ b.next_zero_area(0, 8, align8),
+ b.next_zero_area_off(0, 8, align8, 0)
+ );
+ Ok(())
+ }
+
+ #[test]
+ #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
+ fn owned_bitmap_area_out_of_bounds() -> Result<(), AllocError> {
+ let mut b = BitmapVec::new(64, GFP_KERNEL)?;
+
+ // Should be ignored since out of bounds.
+ b.set(64, 4);
+ b.set(62, 8);
+ b.set(usize::MAX, 0);
+ b.clear(usize::MAX, 0);
+ b.clear(2048, 8);
+ assert_eq!(None, b.next_bit(0));
+ assert_eq!(None, b.next_zero_area(64, 1, Alignment::new::<1>()));
+ Ok(())
+ }
}
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 4/5] rust: id_pool: add contiguous area allocation
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
` (2 preceding siblings ...)
2026-08-12 8:51 ` [PATCH v5 3/5] rust: bitmap: add contiguous area operations Eliot Courtney
@ 2026-08-12 8:51 ` Eliot Courtney
2026-08-12 21:16 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool Eliot Courtney
4 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
Add support for contiguous area allocation. Add a new type,
`UnusedArea`, following the same pattern as `UnusedId`.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/id_pool.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
index 384753fe0e44..eb911a0e3217 100644
--- a/rust/kernel/id_pool.rs
+++ b/rust/kernel/id_pool.rs
@@ -4,8 +4,14 @@
//! Rust API for an ID pool backed by a [`BitmapVec`].
+use core::{
+ num::NonZero,
+ ops::Range, //
+};
+
use crate::alloc::{AllocError, Flags};
use crate::bitmap::BitmapVec;
+use crate::ptr::Alignment;
/// Represents a dynamic ID pool backed by a [`BitmapVec`].
///
@@ -240,6 +246,33 @@ pub fn find_unused_id(&mut self, offset: usize) -> Option<UnusedId<'_>> {
pub fn release_id(&mut self, id: usize) {
self.map.clear_bit(id);
}
+
+ /// Finds a contiguous area of `count` unused IDs at or after `offset`.
+ ///
+ /// The start of the returned area is a multiple of `align`.
+ ///
+ /// Returns an [`UnusedArea`] upon success, or [`None`] if no such area could be found.
+ #[inline]
+ #[must_use]
+ pub fn find_unused_area(
+ &mut self,
+ offset: usize,
+ count: NonZero<usize>,
+ align: Alignment,
+ ) -> Option<UnusedArea<'_>> {
+ let start = self.map.next_zero_area(offset, count.get(), align)?;
+ // INVARIANT: `next_zero_area()` returns None or a start with `start + count <= map.len()`.
+ Some(UnusedArea {
+ range: start..start + count.get(),
+ pool: self,
+ })
+ }
+
+ /// Releases a contiguous area of IDs.
+ #[inline]
+ pub fn release_area(&mut self, range: &Range<usize>) {
+ self.map.clear(range.start, range.len());
+ }
}
/// Represents an unused id in an [`IdPool`].
@@ -287,6 +320,42 @@ pub fn acquire(self) -> usize {
}
}
+/// Represents an unused, contiguous area of IDs in an [`IdPool`].
+///
+/// # Invariants
+///
+/// `range.start <= range.end <= pool.map.len()`.
+#[must_use = "the ID range is not reserved unless acquired"]
+pub struct UnusedArea<'pool> {
+ range: Range<usize>,
+ pool: &'pool mut IdPool,
+}
+
+impl<'pool> UnusedArea<'pool> {
+ /// Returns the unused ID range.
+ ///
+ /// Be aware that the area has not yet been acquired in the pool. The
+ /// [`acquire`] method must be called to prevent others from taking it.
+ ///
+ /// [`acquire`]: UnusedArea::acquire()
+ #[inline]
+ #[must_use]
+ pub fn range(&self) -> Range<usize> {
+ self.range.clone()
+ }
+
+ /// Acquires the area.
+ ///
+ /// Returns the now-reserved ID range.
+ #[inline]
+ pub fn acquire(self) -> Range<usize> {
+ let Self { range, pool } = self;
+ // By the type invariants, the range is within bounds.
+ pool.map.set(range.start, range.end - range.start);
+ range
+ }
+}
+
impl Default for IdPool {
#[inline]
fn default() -> Self {
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
` (3 preceding siblings ...)
2026-08-12 8:51 ` [PATCH v5 4/5] rust: id_pool: add contiguous area allocation Eliot Courtney
@ 2026-08-12 8:51 ` Eliot Courtney
2026-08-12 22:18 ` Yury Norov
4 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-12 8:51 UTC (permalink / raw)
To: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
Eliot Courtney
Add `ChannelIdPool` which adds automatic tracking and releasing of
channel IDs on top of `IdPool`. This is necessary for apportioning
ranges of channel IDs to be used in e.g. vGPU.
Channel IDs are allocated as a contiguous sequence with a specific
length and sometimes a specific alignment [1] for vGPU. The ID space is
small (limited to 2048) and allocation is not on a hot path, so a
bitmap-backed `IdPool` is a better fit than IDA/xarray (which allocate a
single ID within a range, not a contiguous sequence) or a maple tree
(where aligned allocation needs an alloc_range()+erase() retry loop that
essentially reimplements bitmap_find_next_zero_area()) [2]. It is
also faster than maple tree [3].
Link: https://lore.kernel.org/all/84bc8bd2-e292-4b84-9580-a1b5df4c5bdc@nvidia.com/ # [1]
Link: https://lore.kernel.org/all/20260710-chid-maple-v1-1-4ee869055268@nvidia.com/ # [2]
Link: https://lore.kernel.org/all/20260717053241.916441-1-ynorov@nvidia.com/ # [3]
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 2 +
drivers/gpu/nova-core/gpu/channel.rs | 180 +++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 42a4cd7971fa..66ea697a89f8 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -33,6 +33,8 @@
vgpu::VgpuManager, //
};
+#[cfg_attr(not(CONFIG_KUNIT = "y"), expect(dead_code))]
+mod channel;
mod hal;
macro_rules! define_chipset {
diff --git a/drivers/gpu/nova-core/gpu/channel.rs b/drivers/gpu/nova-core/gpu/channel.rs
new file mode 100644
index 000000000000..b755d2184aee
--- /dev/null
+++ b/drivers/gpu/nova-core/gpu/channel.rs
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+//! Channel ID allocation.
+
+use core::{
+ num::NonZero,
+ ops::{
+ Deref,
+ Range, //
+ }, //
+};
+
+use kernel::{
+ id_pool::IdPool,
+ prelude::*,
+ ptr::Alignment,
+ sync::{
+ new_mutex,
+ Mutex, //
+ }, //
+};
+
+/// Pool for tracking reservations of channel IDs.
+#[pin_data]
+pub(crate) struct ChannelIdPool {
+ #[pin]
+ inner: Mutex<IdPool>,
+ num_chids: usize,
+}
+
+impl ChannelIdPool {
+ /// Creates a pool managing `num_chids` channel IDs.
+ pub(crate) fn new(num_chids: usize) -> impl PinInit<Self, Error> {
+ try_pin_init!(Self {
+ inner <- new_mutex!(IdPool::with_capacity(num_chids, GFP_KERNEL)?),
+ num_chids,
+ })
+ }
+
+ /// Reserves a contiguous area of `count` channel IDs starting at a multiple of `align`,
+ /// returning a guard that releases the area on drop.
+ pub(crate) fn alloc_area(
+ &self,
+ count: NonZero<usize>,
+ align: Alignment,
+ ) -> Result<ChannelIdArea<'_>> {
+ let mut ids = self.inner.lock();
+ let area = ids.find_unused_area(0, count, align).ok_or(ENOSPC)?;
+
+ // If the pool is small, the backing bitmap may be rounded up to a larger size.
+ if area.range().end > self.num_chids {
+ return Err(ENOSPC);
+ }
+ Ok(ChannelIdArea {
+ pool: self,
+ range: area.acquire(),
+ })
+ }
+}
+
+/// A reserved contiguous area of channel IDs.
+///
+/// Releases the whole area back to its [`ChannelIdPool`] when dropped. Releasing locks a
+/// sleeping [`Mutex`], so the area must be dropped in a context that is allowed to sleep.
+#[must_use = "the channel ID area is released immediately when unused"]
+pub(crate) struct ChannelIdArea<'a> {
+ pool: &'a ChannelIdPool,
+ range: Range<usize>,
+}
+
+impl Drop for ChannelIdArea<'_> {
+ fn drop(&mut self) {
+ self.pool.inner.lock().release_area(&self.range);
+ }
+}
+
+impl Deref for ChannelIdArea<'_> {
+ type Target = Range<usize>;
+
+ fn deref(&self) -> &Self::Target {
+ &self.range
+ }
+}
+
+#[kunit_tests(nova_core_channel)]
+mod tests {
+ use super::*;
+
+ const fn nz<const N: usize>() -> NonZero<usize> {
+ const { NonZero::new(N).unwrap() }
+ }
+
+ #[test]
+ fn chid_area() -> Result {
+ let pool = KBox::pin_init(ChannelIdPool::new(2048), GFP_KERNEL)?;
+ let unaligned = Alignment::new::<1>();
+
+ let first = pool.alloc_area(nz::<48>(), unaligned)?;
+ assert_eq!(0, first.start);
+ assert_eq!(48, first.len());
+ assert_eq!(48, first.end);
+
+ let second = pool.alloc_area(nz::<48>(), unaligned)?;
+ assert!(first.end <= second.start || second.end <= first.start);
+
+ let first_start = first.start;
+ drop(first);
+ assert_eq!(first_start, pool.alloc_area(nz::<48>(), unaligned)?.start);
+ Ok(())
+ }
+
+ #[test]
+ fn chid_bounded_by_num_chids() -> Result {
+ let pool = KBox::pin_init(ChannelIdPool::new(4), GFP_KERNEL)?;
+ let unaligned = Alignment::new::<1>();
+
+ {
+ let a = pool.alloc_area(nz::<1>(), unaligned)?;
+ let b = pool.alloc_area(nz::<1>(), unaligned)?;
+ let c = pool.alloc_area(nz::<1>(), unaligned)?;
+ let d = pool.alloc_area(nz::<1>(), unaligned)?;
+ assert_eq!(0, a.start);
+ assert_eq!(1, b.start);
+ assert_eq!(2, c.start);
+ assert_eq!(3, d.start);
+ assert_eq!(
+ Err(ENOSPC),
+ pool.alloc_area(nz::<1>(), unaligned).map(|_| ())
+ );
+ }
+
+ assert_eq!(0, pool.alloc_area(nz::<4>(), unaligned)?.start);
+ assert_eq!(
+ Err(ENOSPC),
+ pool.alloc_area(nz::<5>(), unaligned).map(|_| ())
+ );
+
+ let head = pool.alloc_area(nz::<3>(), unaligned)?;
+ assert_eq!(0, head.start);
+ assert_eq!(
+ Err(ENOSPC),
+ pool.alloc_area(nz::<2>(), unaligned).map(|_| ())
+ );
+ assert_eq!(3, pool.alloc_area(nz::<1>(), unaligned)?.start);
+ Ok(())
+ }
+
+ #[test]
+ fn chid_area_aligned() -> Result {
+ let pool = KBox::pin_init(ChannelIdPool::new(16), GFP_KERNEL)?;
+ let unaligned = Alignment::new::<1>();
+ let align4 = Alignment::new::<4>();
+
+ // Alloc 0 so the first fit for the next area is unaligned.
+ let pad = pool.alloc_area(nz::<1>(), unaligned)?;
+ assert_eq!(0, pad.start);
+
+ let a = pool.alloc_area(nz::<4>(), align4)?;
+ assert_eq!(4, a.start);
+
+ // The area skipped over by the aligned allocation should still be available.
+ let b = pool.alloc_area(nz::<1>(), unaligned)?;
+ assert_eq!(1, b.start);
+
+ let c = pool.alloc_area(nz::<8>(), Alignment::new::<8>())?;
+ assert_eq!(8, c.start);
+
+ // Only 2 IDs left.
+ assert_eq!(Err(ENOSPC), pool.alloc_area(nz::<4>(), align4).map(|_| ()));
+ assert_eq!(
+ Err(ENOSPC),
+ pool.alloc_area(nz::<1>(), Alignment::new::<32>())
+ .map(|_| ())
+ );
+
+ assert_eq!(2, pool.alloc_area(nz::<2>(), unaligned)?.start);
+ Ok(())
+ }
+}
--
2.55.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX
2026-08-12 8:51 ` [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
@ 2026-08-12 19:44 ` Yury Norov
0 siblings, 0 replies; 15+ messages in thread
From: Yury Norov @ 2026-08-12 19:44 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel
On Wed, Aug 12, 2026 at 05:51:22PM +0900, Eliot Courtney wrote:
> It is currently possible to construct a non-`BitmapVec` backed
> `Bitmap` using `Bitmap::from_raw` that is larger than `i32::MAX`, and
> it is not part of the unsafe requirements. Restricting all bitmaps
> (even non-`BitmapVec` backed ones) to a maximum size of `i32::MAX`
> simplifies a few things and matches `BitmapVec::MAX_LEN`.
>
> Add that requirement to the unsafe requirements on `Bitmap::from_raw`
> and `Bitmap::from_raw_mut`, and to the invariants on `Bitmap`.
>
> This also fixes u32 casts truncating in `copy_and_extend`, which could
> otherwise lead to OOB writes.
>
> Fixes: 11eca92a2cae ("rust: add bitmap API.")
> Link: https://lore.kernel.org/DKG0U8RLO7LZ.2I1AIH0S38PAP@nvidia.com
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Yury Norov <ynorov@nvidia.com>
> ---
> rust/kernel/bitmap.rs | 68 +++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index a43bfe0ec3dc..fdcfc0409773 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs
> @@ -17,24 +17,57 @@
> /// # Invariants
> ///
> /// Must reference a `[c_ulong]` long enough to fit `data.len()` bits.
> +/// Must not be longer than `i32::MAX` bits.
> #[cfg_attr(CONFIG_64BIT, repr(align(8)))]
> #[cfg_attr(not(CONFIG_64BIT), repr(align(4)))]
> pub struct Bitmap {
> data: [()],
> }
>
> +macro_rules! bitmap_assert {
> + ($cond:expr, $($arg:tt)+) => {
> + #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> + assert!($cond, $($arg)*);
> + }
> +}
> +
> +macro_rules! bitmap_assert_return {
> + ($cond:expr, $($arg:tt)+) => {
> + #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> + assert!($cond, $($arg)*);
> +
> + #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> + if !($cond) {
> + pr_err!($($arg)*);
> + return
> + }
> + }
> +}
> +
> impl Bitmap {
> /// Borrows a C bitmap.
> ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds `i32::MAX`.
> + ///
> /// # Safety
> ///
> /// * `ptr` holds a non-null address of an initialized array of `unsigned long`
> /// that is large enough to hold `nbits` bits.
> + /// * `nbits` must not exceed `i32::MAX`.
> /// * the array must not be freed for the lifetime of this [`Bitmap`]
> /// * concurrent access only happens through atomic operations
> pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a Bitmap {
> + bitmap_assert!(
> + nbits <= i32::MAX as usize,
> + "`nbits` must be <= {}, was {}",
> + i32::MAX,
> + nbits
> + );
> let data: *const [()] = core::ptr::slice_from_raw_parts(ptr.cast(), nbits);
> // INVARIANT: `data` references an initialized array that can hold `nbits` bits.
> + // INVARIANT: the caller guarantees that `nbits` does not exceed `i32::MAX`.
> // SAFETY:
> // The caller guarantees that `data` (derived from `ptr` and `nbits`)
> // points to a valid, initialized, and appropriately sized memory region
> @@ -51,15 +84,27 @@ pub unsafe fn from_raw<'a>(ptr: *const usize, nbits: usize) -> &'a Bitmap {
>
> /// Borrows a C bitmap exclusively.
> ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `nbits` exceeds `i32::MAX`.
> + ///
> /// # Safety
> ///
> /// * `ptr` holds a non-null address of an initialized array of `unsigned long`
> /// that is large enough to hold `nbits` bits.
> + /// * `nbits` must not exceed `i32::MAX`.
> /// * the array must not be freed for the lifetime of this [`Bitmap`]
> /// * no concurrent access may happen.
> pub unsafe fn from_raw_mut<'a>(ptr: *mut usize, nbits: usize) -> &'a mut Bitmap {
> + bitmap_assert!(
> + nbits <= i32::MAX as usize,
> + "`nbits` must be <= {}, was {}",
> + i32::MAX,
> + nbits
> + );
> let data: *mut [()] = core::ptr::slice_from_raw_parts_mut(ptr.cast(), nbits);
> // INVARIANT: `data` references an initialized array that can hold `nbits` bits.
> + // INVARIANT: the caller guarantees that `nbits` does not exceed `i32::MAX`.
> // SAFETY:
> // The caller guarantees that `data` (derived from `ptr` and `nbits`)
> // points to a valid, initialized, and appropriately sized memory region
> @@ -96,26 +141,6 @@ union BitmapRepr {
> ptr: NonNull<usize>,
> }
>
> -macro_rules! bitmap_assert {
> - ($cond:expr, $($arg:tt)+) => {
> - #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> - assert!($cond, $($arg)*);
> - }
> -}
> -
> -macro_rules! bitmap_assert_return {
> - ($cond:expr, $($arg:tt)+) => {
> - #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> - assert!($cond, $($arg)*);
> -
> - #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> - if !($cond) {
> - pr_err!($($arg)*);
> - return
> - }
> - }
> -}
> -
> /// Represents an owned bitmap.
> ///
> /// Wraps underlying C bitmap API. See [`Bitmap`] for available
> @@ -415,7 +440,8 @@ pub fn clear_bit_atomic(&self, index: usize) {
> #[inline]
> pub fn copy_and_extend(&mut self, src: &Bitmap) {
> let len = core::cmp::min(src.len(), self.len());
> - // SAFETY: access to `self` and `src` is within bounds.
> + // SAFETY: access to `self` and `src` is within bounds. Both lengths fit in `u32`
> + // because a `Bitmap` is at most `i32::MAX` bits, so the casts are lossless.
> unsafe {
> bindings::bitmap_copy_and_extend(
> self.as_mut_ptr(),
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/5] rust: bitmap: add contiguous area operations
2026-08-12 8:51 ` [PATCH v5 3/5] rust: bitmap: add contiguous area operations Eliot Courtney
@ 2026-08-12 20:31 ` Yury Norov
2026-08-13 7:27 ` Eliot Courtney
0 siblings, 1 reply; 15+ messages in thread
From: Yury Norov @ 2026-08-12 20:31 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel
On Wed, Aug 12, 2026 at 05:51:23PM +0900, Eliot Courtney wrote:
> Add bindings for area operations on bitmaps. Each one is
> made safe by adding some extra checks compared to the underlying C code
> (for example, checking bounds) and with additional checks to catch
> likely erroneous usage if `CONFIG_RUST_BITMAP_HARDENED` is on.
>
> Add tests demonstrating the edge cases.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> rust/kernel/bitmap.rs | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 236 insertions(+)
>
> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index fdcfc0409773..74c92cc452c9 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs
> @@ -10,6 +10,7 @@
> use crate::bindings;
> #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> use crate::pr_err;
> +use crate::ptr::Alignment;
> use core::ptr::NonNull;
>
> /// Represents a C bitmap. Wraps underlying C bitmap API.
> @@ -523,6 +524,139 @@ pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
> Some(index)
> }
> }
> +
> + /// Finds a contiguous area of `nbits` zero bits at or after `start`, where the area plus
> + /// `align_offset` is aligned to `align`.
> + ///
> + /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
> + /// the bitmap exists.
> + ///
> + /// The returned index plus `align_offset` is a multiple of `align`.
> + ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
> + #[inline]
> + pub fn next_zero_area_off(
> + &self,
> + start: usize,
> + nbits: usize,
> + align: Alignment,
> + align_offset: usize,
> + ) -> Option<usize> {
> + bitmap_assert!(
> + start < self.len(),
> + "`start` must be < {}, was {}",
> + self.len(),
> + start
> + );
> +
> + let nr = u32::try_from(nbits).ok()?;
What about nbits == 0? In C, this is a undef, and thus in the current
rust implementation. Maybe make it NonZero?
The same question about align and align_offset.
> + let align_mask = align.as_usize() - 1;
> +
> + // The C alignment and end arithmetic must not overflow, or it can read out of bounds.
> + // Overflow is only possible on 32-bit.
> + #[cfg(not(CONFIG_64BIT))]
> + align_mask.checked_add(self.len())?.checked_add(nbits)?;
> +
> + // SAFETY: `bitmap_find_next_zero_area_off` is safe to use with an out of bounds `start`
> + // value and, given the overflow check above, never reads beyond `self.len()` bits.
> + let index = unsafe {
> + bindings::bitmap_find_next_zero_area_off(
> + self.as_ptr().cast_mut(),
> + self.len(),
> + start,
> + nr,
> + align_mask,
> + align_offset,
> + )
> + };
> +
> + (index < self.len()).then_some(index)
> + }
> +
> + /// Finds a contiguous area of `nbits` zero bits at or after `start`, aligned to `align`.
> + ///
> + /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
> + /// the bitmap exists.
> + ///
> + /// The returned index is a multiple of `align`.
> + ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
> + /// use kernel::bitmap::BitmapVec;
> + /// use kernel::ptr::Alignment;
> + ///
> + /// let mut b = BitmapVec::new(64, GFP_KERNEL)?;
> + /// let unaligned = Alignment::new::<1>();
> + ///
> + /// assert_eq!(Some(0), b.next_zero_area(0, 8, unaligned));
> + /// b.set(0, 5);
> + /// assert_eq!(Some(5), b.next_zero_area(0, 8, unaligned));
> + /// assert_eq!(Some(8), b.next_zero_area(0, 8, Alignment::new::<8>()));
> + /// assert_eq!(None, b.next_zero_area(0, 65, unaligned));
> + /// # Ok::<(), AllocError>(())
> + /// ```
> + #[inline]
> + pub fn next_zero_area(&self, start: usize, nbits: usize, align: Alignment) -> Option<usize> {
> + self.next_zero_area_off(start, nbits, align, 0)
> + }
> +
> + /// Sets a contiguous area of `nbits` bits starting at `start`.
> + ///
> + /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
> + /// bounds, does nothing.
> + ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
> + /// of bounds.
> + #[inline]
> + pub fn set(&mut self, start: usize, nbits: usize) {
> + bitmap_assert_return!(
> + start
> + .checked_add(nbits)
> + .is_some_and(|end| end <= self.len()),
> + "Area `start..start + nbits` ({}..{}) must be within bounds {}",
> + start,
> + start.saturating_add(nbits),
> + self.len()
> + );
> + // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
> + // `i32::MAX` bits, so the casts are lossless.
> + unsafe { bindings::__bitmap_set(self.as_mut_ptr(), start as u32, nbits as i32) };
> + }
In the case of bitmap_set/clear(), nbits == 0 makes it a no-op, and
guarantees that the pointer is not dereferenced. So, no undefined
behavior. But in rust case, I believe, it should be a stronger policy.
I'd add an assertion, at least, or better make it NonZero.
Thanks,
Yury
> +
> + /// Clears a contiguous area of `nbits` bits starting at `start`.
> + ///
> + /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
> + /// bounds, does nothing.
> + ///
> + /// # Panics
> + ///
> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
> + /// of bounds.
> + #[inline]
> + pub fn clear(&mut self, start: usize, nbits: usize) {
> + bitmap_assert_return!(
> + start
> + .checked_add(nbits)
> + .is_some_and(|end| end <= self.len()),
> + "Area `start..start + nbits` ({}..{}) must be within bounds {}",
> + start,
> + start.saturating_add(nbits),
> + self.len()
> + );
> + // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
> + // `i32::MAX` bits, so the casts are lossless.
> + unsafe { bindings::__bitmap_clear(self.as_mut_ptr(), start as u32, nbits as i32) };
> + }
> }
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 4/5] rust: id_pool: add contiguous area allocation
2026-08-12 8:51 ` [PATCH v5 4/5] rust: id_pool: add contiguous area allocation Eliot Courtney
@ 2026-08-12 21:16 ` Yury Norov
2026-08-13 7:29 ` Eliot Courtney
0 siblings, 1 reply; 15+ messages in thread
From: Yury Norov @ 2026-08-12 21:16 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel
On Wed, Aug 12, 2026 at 05:51:24PM +0900, Eliot Courtney wrote:
> Add support for contiguous area allocation. Add a new type,
> `UnusedArea`, following the same pattern as `UnusedId`.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> rust/kernel/id_pool.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index 384753fe0e44..eb911a0e3217 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -4,8 +4,14 @@
>
> //! Rust API for an ID pool backed by a [`BitmapVec`].
>
> +use core::{
> + num::NonZero,
> + ops::Range, //
> +};
> +
> use crate::alloc::{AllocError, Flags};
> use crate::bitmap::BitmapVec;
> +use crate::ptr::Alignment;
>
> /// Represents a dynamic ID pool backed by a [`BitmapVec`].
> ///
> @@ -240,6 +246,33 @@ pub fn find_unused_id(&mut self, offset: usize) -> Option<UnusedId<'_>> {
> pub fn release_id(&mut self, id: usize) {
> self.map.clear_bit(id);
> }
> +
> + /// Finds a contiguous area of `count` unused IDs at or after `offset`.
> + ///
> + /// The start of the returned area is a multiple of `align`.
> + ///
> + /// Returns an [`UnusedArea`] upon success, or [`None`] if no such area could be found.
> + #[inline]
> + #[must_use]
> + pub fn find_unused_area(
> + &mut self,
> + offset: usize,
> + count: NonZero<usize>,
> + align: Alignment,
> + ) -> Option<UnusedArea<'_>> {
> + let start = self.map.next_zero_area(offset, count.get(), align)?;
> + // INVARIANT: `next_zero_area()` returns None or a start with `start + count <= map.len()`.
> + Some(UnusedArea {
> + range: start..start + count.get(),
> + pool: self,
> + })
> + }
> +
> + /// Releases a contiguous area of IDs.
> + #[inline]
> + pub fn release_area(&mut self, range: &Range<usize>) {
> + self.map.clear(range.start, range.len());
> + }
> }
>
> /// Represents an unused id in an [`IdPool`].
> @@ -287,6 +320,42 @@ pub fn acquire(self) -> usize {
> }
> }
>
> +/// Represents an unused, contiguous area of IDs in an [`IdPool`].
> +///
> +/// # Invariants
> +///
> +/// `range.start <= range.end <= pool.map.len()`.
> +#[must_use = "the ID range is not reserved unless acquired"]
> +pub struct UnusedArea<'pool> {
> + range: Range<usize>,
> + pool: &'pool mut IdPool,
> +}
So, the compilation message refers the "ID range", not the UnusedArea.
To me, this 'unused' language is confusing. What should I do with the
area that I just allocated? Drop the 'unused' one and create the 'used'?
Can you rename it to id_range please? Then the API would look more
consistent, at least to me.
> +
> +impl<'pool> UnusedArea<'pool> {
> + /// Returns the unused ID range.
> + ///
> + /// Be aware that the area has not yet been acquired in the pool. The
> + /// [`acquire`] method must be called to prevent others from taking it.
> + ///
> + /// [`acquire`]: UnusedArea::acquire()
So maybe implement the find_acquire() method? In the caller you
serialize it with:
let mut ids = self.inner.lock();
Is it possible to pass this down to the suggested find_acquire()? In
my experience, having non-atomic sequence of find + acquire that
requires the external locking is the recipe for troubles.
> + #[inline]
> + #[must_use]
> + pub fn range(&self) -> Range<usize> {
> + self.range.clone()
> + }
> +
> + /// Acquires the area.
> + ///
> + /// Returns the now-reserved ID range.
> + #[inline]
> + pub fn acquire(self) -> Range<usize> {
> + let Self { range, pool } = self;
> + // By the type invariants, the range is within bounds.
> + pool.map.set(range.start, range.end - range.start);
> + range
From hierarchy perspective, the UnusedArea wraps the Range, and
passing the Range to the higher layer breaks the hierarchy. If you
follow my suggestion, the hierarchy will be enforced stricter:
ChannelIdRange -> IdRange-> Range
instead of
ChannelIdArea -> UnusedArea-> Range
|
-> Range
Or I misunderstand the concept of the UnusedArea?
> + }
> +}
> +
> impl Default for IdPool {
> #[inline]
> fn default() -> Self {
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool
2026-08-12 8:51 ` [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool Eliot Courtney
@ 2026-08-12 22:18 ` Yury Norov
2026-08-13 7:31 ` Eliot Courtney
0 siblings, 1 reply; 15+ messages in thread
From: Yury Norov @ 2026-08-12 22:18 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel
On Wed, Aug 12, 2026 at 05:51:25PM +0900, Eliot Courtney wrote:
> Add `ChannelIdPool` which adds automatic tracking and releasing of
> channel IDs on top of `IdPool`. This is necessary for apportioning
> ranges of channel IDs to be used in e.g. vGPU.
>
> Channel IDs are allocated as a contiguous sequence with a specific
> length and sometimes a specific alignment [1] for vGPU. The ID space is
> small (limited to 2048) and allocation is not on a hot path, so a
> bitmap-backed `IdPool` is a better fit than IDA/xarray (which allocate a
> single ID within a range, not a contiguous sequence) or a maple tree
> (where aligned allocation needs an alloc_range()+erase() retry loop that
> essentially reimplements bitmap_find_next_zero_area()) [2]. It is
> also faster than maple tree [3].
>
> Link: https://lore.kernel.org/all/84bc8bd2-e292-4b84-9580-a1b5df4c5bdc@nvidia.com/ # [1]
> Link: https://lore.kernel.org/all/20260710-chid-maple-v1-1-4ee869055268@nvidia.com/ # [2]
> Link: https://lore.kernel.org/all/20260717053241.916441-1-ynorov@nvidia.com/ # [3]
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> drivers/gpu/nova-core/gpu.rs | 2 +
> drivers/gpu/nova-core/gpu/channel.rs | 180 +++++++++++++++++++++++++++++++++++
> 2 files changed, 182 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 42a4cd7971fa..66ea697a89f8 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -33,6 +33,8 @@
> vgpu::VgpuManager, //
> };
>
> +#[cfg_attr(not(CONFIG_KUNIT = "y"), expect(dead_code))]
> +mod channel;
> mod hal;
>
> macro_rules! define_chipset {
> diff --git a/drivers/gpu/nova-core/gpu/channel.rs b/drivers/gpu/nova-core/gpu/channel.rs
> new file mode 100644
> index 000000000000..b755d2184aee
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gpu/channel.rs
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! Channel ID allocation.
> +
> +use core::{
> + num::NonZero,
> + ops::{
> + Deref,
> + Range, //
> + }, //
> +};
> +
> +use kernel::{
> + id_pool::IdPool,
> + prelude::*,
> + ptr::Alignment,
> + sync::{
> + new_mutex,
> + Mutex, //
> + }, //
> +};
> +
> +/// Pool for tracking reservations of channel IDs.
> +#[pin_data]
> +pub(crate) struct ChannelIdPool {
> + #[pin]
> + inner: Mutex<IdPool>,
> + num_chids: usize,
> +}
> +
> +impl ChannelIdPool {
> + /// Creates a pool managing `num_chids` channel IDs.
> + pub(crate) fn new(num_chids: usize) -> impl PinInit<Self, Error> {
> + try_pin_init!(Self {
> + inner <- new_mutex!(IdPool::with_capacity(num_chids, GFP_KERNEL)?),
> + num_chids,
> + })
> + }
> +
> + /// Reserves a contiguous area of `count` channel IDs starting at a multiple of `align`,
> + /// returning a guard that releases the area on drop.
> + pub(crate) fn alloc_area(
> + &self,
> + count: NonZero<usize>,
OK, here you use NonZero. Please do that in the lowest layer.
> + align: Alignment,
> + ) -> Result<ChannelIdArea<'_>> {
> + let mut ids = self.inner.lock();
> + let area = ids.find_unused_area(0, count, align).ok_or(ENOSPC)?;
> +
> + // If the pool is small, the backing bitmap may be rounded up to a larger size.
Not sure I understand this language. Your ID pool is a fixed-size. Or
do you mean something else?
> + if area.range().end > self.num_chids {
> + return Err(ENOSPC);
> + }
> + Ok(ChannelIdArea {
> + pool: self,
> + range: area.acquire(),
> + })
> + }
> +}
> +
> +/// A reserved contiguous area of channel IDs.
> +///
> +/// Releases the whole area back to its [`ChannelIdPool`] when dropped. Releasing locks a
> +/// sleeping [`Mutex`], so the area must be dropped in a context that is allowed to sleep.
> +#[must_use = "the channel ID area is released immediately when unused"]
> +pub(crate) struct ChannelIdArea<'a> {
> + pool: &'a ChannelIdPool,
> + range: Range<usize>,
> +}
> +
> +impl Drop for ChannelIdArea<'_> {
> + fn drop(&mut self) {
> + self.pool.inner.lock().release_area(&self.range);
> + }
> +}
> +
> +impl Deref for ChannelIdArea<'_> {
> + type Target = Range<usize>;
> +
> + fn deref(&self) -> &Self::Target {
> + &self.range
> + }
> +}
> +
> +#[kunit_tests(nova_core_channel)]
> +mod tests {
> + use super::*;
> +
> + const fn nz<const N: usize>() -> NonZero<usize> {
> + const { NonZero::new(N).unwrap() }
> + }
> +
> + #[test]
> + fn chid_area() -> Result {
> + let pool = KBox::pin_init(ChannelIdPool::new(2048), GFP_KERNEL)?;
> + let unaligned = Alignment::new::<1>();
> +
> + let first = pool.alloc_area(nz::<48>(), unaligned)?;
> + assert_eq!(0, first.start);
> + assert_eq!(48, first.len());
> + assert_eq!(48, first.end);
> +
> + let second = pool.alloc_area(nz::<48>(), unaligned)?;
> + assert!(first.end <= second.start || second.end <= first.start);
> +
> + let first_start = first.start;
> + drop(first);
You test the drop() only once. Can you add more tests? At least, make
sure that 2 allocs followed by 2 drops ends up with an empty pool.
> + assert_eq!(first_start, pool.alloc_area(nz::<48>(), unaligned)?.start);
> + Ok(())
> + }
> +
> + #[test]
> + fn chid_bounded_by_num_chids() -> Result {
> + let pool = KBox::pin_init(ChannelIdPool::new(4), GFP_KERNEL)?;
> + let unaligned = Alignment::new::<1>();
> +
> + {
> + let a = pool.alloc_area(nz::<1>(), unaligned)?;
> + let b = pool.alloc_area(nz::<1>(), unaligned)?;
> + let c = pool.alloc_area(nz::<1>(), unaligned)?;
> + let d = pool.alloc_area(nz::<1>(), unaligned)?;
OK, here your alloc_area() means the find + alloc, and it returns
a Range - not area.
To me it looks like the intermediate UnusedArea layer is excessive.
If you just do find + alloc in this pool.alloc_area(), you seemingly
don't need the UnusedArea.
Can you try without it, please?
> + assert_eq!(0, a.start);
> + assert_eq!(1, b.start);
> + assert_eq!(2, c.start);
> + assert_eq!(3, d.start);
> + assert_eq!(
> + Err(ENOSPC),
> + pool.alloc_area(nz::<1>(), unaligned).map(|_| ())
> + );
> + }
> +
> + assert_eq!(0, pool.alloc_area(nz::<4>(), unaligned)?.start);
> + assert_eq!(
> + Err(ENOSPC),
> + pool.alloc_area(nz::<5>(), unaligned).map(|_| ())
> + );
> +
> + let head = pool.alloc_area(nz::<3>(), unaligned)?;
> + assert_eq!(0, head.start);
> + assert_eq!(
> + Err(ENOSPC),
> + pool.alloc_area(nz::<2>(), unaligned).map(|_| ())
> + );
> + assert_eq!(3, pool.alloc_area(nz::<1>(), unaligned)?.start);
> + Ok(())
> + }
> +
> + #[test]
> + fn chid_area_aligned() -> Result {
> + let pool = KBox::pin_init(ChannelIdPool::new(16), GFP_KERNEL)?;
> + let unaligned = Alignment::new::<1>();
> + let align4 = Alignment::new::<4>();
> +
> + // Alloc 0 so the first fit for the next area is unaligned.
> + let pad = pool.alloc_area(nz::<1>(), unaligned)?;
> + assert_eq!(0, pad.start);
> +
> + let a = pool.alloc_area(nz::<4>(), align4)?;
> + assert_eq!(4, a.start);
> +
> + // The area skipped over by the aligned allocation should still be available.
> + let b = pool.alloc_area(nz::<1>(), unaligned)?;
> + assert_eq!(1, b.start);
> +
> + let c = pool.alloc_area(nz::<8>(), Alignment::new::<8>())?;
Is it possible to make it somehow simpler:
let c = pool.alloc_area(8, 8)?;
All the parameters checking must be a part of implementations, not the
interface.
We had a very similar discussion in the bitfields implementation thread,
and many people in CC list of this thread spent quite a long time to find
a way from:
let color = Rgb::default()
.set_red(Bounded::<u16, _>::new::<0x10>())
.set_green(Bounded::<u16, _>::new::<0x1f>())
.set_blue(Bounded::<u16, _>::new::<0x18>());
to:
let color = Rgb::default().
.set_red(0x10)
.set_green(0x1f)
.set_blue(0x18)
Can you do the same here? Please refer:
https://lore.kernel.org/all/aXCZeVqkDrBWr1uq@yury/
> + assert_eq!(8, c.start);
> +
> + // Only 2 IDs left.
> + assert_eq!(Err(ENOSPC), pool.alloc_area(nz::<4>(), align4).map(|_| ()));
> + assert_eq!(
> + Err(ENOSPC),
> + pool.alloc_area(nz::<1>(), Alignment::new::<32>())
> + .map(|_| ())
> + );
> +
> + assert_eq!(2, pool.alloc_area(nz::<2>(), unaligned)?.start);
> + Ok(())
> + }
> +}
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test
2026-08-12 8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
@ 2026-08-12 22:23 ` Yury Norov
0 siblings, 0 replies; 15+ messages in thread
From: Yury Norov @ 2026-08-12 22:23 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel
On Wed, Aug 12, 2026 at 05:51:21PM +0900, Eliot Courtney wrote:
> Since commit c652dc44192d ("rust: kunit: allow `cfg` on `test`s"),
> we no longer need this workaround.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
Reviewed-by: Yury Norov <ynorov@nvidia.com>
> ---
> rust/kernel/bitmap.rs | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> index b27e0ec80d64..a43bfe0ec3dc 100644
> --- a/rust/kernel/bitmap.rs
> +++ b/rust/kernel/bitmap.rs
> @@ -572,24 +572,21 @@ fn bitmap_set_clear_find() -> Result<(), AllocError> {
> }
>
> #[test]
> + #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> fn owned_bitmap_out_of_bounds() -> Result<(), AllocError> {
> - // TODO: Kunit #[test]s do not support `cfg` yet,
> - // so we add it here in the body.
> - #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
> - {
> - let mut b = BitmapVec::new(128, GFP_KERNEL)?;
> - b.set_bit(2048);
> - b.set_bit_atomic(2048);
> - b.clear_bit(2048);
> - b.clear_bit_atomic(2048);
> - assert_eq!(None, b.next_bit(2048));
> - assert_eq!(None, b.next_zero_bit(2048));
> - assert_eq!(None, b.last_bit());
> - }
> + let mut b = BitmapVec::new(128, GFP_KERNEL)?;
> +
> + b.set_bit(2048);
> + b.set_bit_atomic(2048);
> + b.clear_bit(2048);
> + b.clear_bit_atomic(2048);
> + assert_eq!(None, b.next_bit(2048));
> + assert_eq!(None, b.next_zero_bit(2048));
> + assert_eq!(None, b.last_bit());
> Ok(())
> }
>
> - // TODO: uncomment once kunit supports [should_panic] and `cfg`.
> + // TODO: uncomment once kunit supports `#[should_panic]`.
> // #[cfg(CONFIG_RUST_BITMAP_HARDENED)]
> // #[test]
> // #[should_panic]
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/5] rust: bitmap: add contiguous area operations
2026-08-12 20:31 ` Yury Norov
@ 2026-08-13 7:27 ` Eliot Courtney
0 siblings, 0 replies; 15+ messages in thread
From: Eliot Courtney @ 2026-08-13 7:27 UTC (permalink / raw)
To: Yury Norov, Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
dri-devel
On Thu Aug 13, 2026 at 5:31 AM JST, Yury Norov wrote:
> On Wed, Aug 12, 2026 at 05:51:23PM +0900, Eliot Courtney wrote:
>> Add bindings for area operations on bitmaps. Each one is
>> made safe by adding some extra checks compared to the underlying C code
>> (for example, checking bounds) and with additional checks to catch
>> likely erroneous usage if `CONFIG_RUST_BITMAP_HARDENED` is on.
>>
>> Add tests demonstrating the edge cases.
>>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>> rust/kernel/bitmap.rs | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 236 insertions(+)
>>
>> diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
>> index fdcfc0409773..74c92cc452c9 100644
>> --- a/rust/kernel/bitmap.rs
>> +++ b/rust/kernel/bitmap.rs
>> @@ -10,6 +10,7 @@
>> use crate::bindings;
>> #[cfg(not(CONFIG_RUST_BITMAP_HARDENED))]
>> use crate::pr_err;
>> +use crate::ptr::Alignment;
>> use core::ptr::NonNull;
>>
>> /// Represents a C bitmap. Wraps underlying C bitmap API.
>> @@ -523,6 +524,139 @@ pub fn next_zero_bit(&self, start: usize) -> Option<usize> {
>> Some(index)
>> }
>> }
>> +
>> + /// Finds a contiguous area of `nbits` zero bits at or after `start`, where the area plus
>> + /// `align_offset` is aligned to `align`.
>> + ///
>> + /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
>> + /// the bitmap exists.
>> + ///
>> + /// The returned index plus `align_offset` is a multiple of `align`.
>> + ///
>> + /// # Panics
>> + ///
>> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
>> + #[inline]
>> + pub fn next_zero_area_off(
>> + &self,
>> + start: usize,
>> + nbits: usize,
>> + align: Alignment,
>> + align_offset: usize,
>> + ) -> Option<usize> {
>> + bitmap_assert!(
>> + start < self.len(),
>> + "`start` must be < {}, was {}",
>> + self.len(),
>> + start
>> + );
>> +
>> + let nr = u32::try_from(nbits).ok()?;
>
> What about nbits == 0? In C, this is a undef, and thus in the current
> rust implementation. Maybe make it NonZero?
>
> The same question about align and align_offset.
NonZero sounds good to me for `nbits`. For `align`, it's already
guaranteed to be at least 1. For `align_offset`, passing 0 is normal and
valid (and we need to for implementing `next_zero_area` just below)
>
>> + let align_mask = align.as_usize() - 1;
>> +
>> + // The C alignment and end arithmetic must not overflow, or it can read out of bounds.
>> + // Overflow is only possible on 32-bit.
>> + #[cfg(not(CONFIG_64BIT))]
>> + align_mask.checked_add(self.len())?.checked_add(nbits)?;
>> +
>> + // SAFETY: `bitmap_find_next_zero_area_off` is safe to use with an out of bounds `start`
>> + // value and, given the overflow check above, never reads beyond `self.len()` bits.
>> + let index = unsafe {
>> + bindings::bitmap_find_next_zero_area_off(
>> + self.as_ptr().cast_mut(),
>> + self.len(),
>> + start,
>> + nr,
>> + align_mask,
>> + align_offset,
>> + )
>> + };
>> +
>> + (index < self.len()).then_some(index)
>> + }
>> +
>> + /// Finds a contiguous area of `nbits` zero bits at or after `start`, aligned to `align`.
>> + ///
>> + /// Returns the bit index of the start of the area, or [`None`] if no such area fitting in
>> + /// the bitmap exists.
>> + ///
>> + /// The returned index is a multiple of `align`.
>> + ///
>> + /// # Panics
>> + ///
>> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and `start` is out of bounds.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```
>> + /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
>> + /// use kernel::bitmap::BitmapVec;
>> + /// use kernel::ptr::Alignment;
>> + ///
>> + /// let mut b = BitmapVec::new(64, GFP_KERNEL)?;
>> + /// let unaligned = Alignment::new::<1>();
>> + ///
>> + /// assert_eq!(Some(0), b.next_zero_area(0, 8, unaligned));
>> + /// b.set(0, 5);
>> + /// assert_eq!(Some(5), b.next_zero_area(0, 8, unaligned));
>> + /// assert_eq!(Some(8), b.next_zero_area(0, 8, Alignment::new::<8>()));
>> + /// assert_eq!(None, b.next_zero_area(0, 65, unaligned));
>> + /// # Ok::<(), AllocError>(())
>> + /// ```
>> + #[inline]
>> + pub fn next_zero_area(&self, start: usize, nbits: usize, align: Alignment) -> Option<usize> {
>> + self.next_zero_area_off(start, nbits, align, 0)
>> + }
>> +
>> + /// Sets a contiguous area of `nbits` bits starting at `start`.
>> + ///
>> + /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
>> + /// bounds, does nothing.
>> + ///
>> + /// # Panics
>> + ///
>> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
>> + /// of bounds.
>> + #[inline]
>> + pub fn set(&mut self, start: usize, nbits: usize) {
>> + bitmap_assert_return!(
>> + start
>> + .checked_add(nbits)
>> + .is_some_and(|end| end <= self.len()),
>> + "Area `start..start + nbits` ({}..{}) must be within bounds {}",
>> + start,
>> + start.saturating_add(nbits),
>> + self.len()
>> + );
>> + // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
>> + // `i32::MAX` bits, so the casts are lossless.
>> + unsafe { bindings::__bitmap_set(self.as_mut_ptr(), start as u32, nbits as i32) };
>> + }
>
> In the case of bitmap_set/clear(), nbits == 0 makes it a no-op, and
> guarantees that the pointer is not dereferenced. So, no undefined
> behavior. But in rust case, I believe, it should be a stronger policy.
>
> I'd add an assertion, at least, or better make it NonZero.
>
> Thanks,
> Yury
Yeah, NonZero sounds good to me here too. Thanks!
>
>> +
>> + /// Clears a contiguous area of `nbits` bits starting at `start`.
>> + ///
>> + /// If CONFIG_RUST_BITMAP_HARDENED is not enabled and the area `start..start + nbits` is out of
>> + /// bounds, does nothing.
>> + ///
>> + /// # Panics
>> + ///
>> + /// Panics if CONFIG_RUST_BITMAP_HARDENED is enabled and the area `start..start + nbits` is out
>> + /// of bounds.
>> + #[inline]
>> + pub fn clear(&mut self, start: usize, nbits: usize) {
>> + bitmap_assert_return!(
>> + start
>> + .checked_add(nbits)
>> + .is_some_and(|end| end <= self.len()),
>> + "Area `start..start + nbits` ({}..{}) must be within bounds {}",
>> + start,
>> + start.saturating_add(nbits),
>> + self.len()
>> + );
>> + // SAFETY: The area `start..start + nbits` is within bounds and a `Bitmap` is at most
>> + // `i32::MAX` bits, so the casts are lossless.
>> + unsafe { bindings::__bitmap_clear(self.as_mut_ptr(), start as u32, nbits as i32) };
>> + }
>> }
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 4/5] rust: id_pool: add contiguous area allocation
2026-08-12 21:16 ` Yury Norov
@ 2026-08-13 7:29 ` Eliot Courtney
0 siblings, 0 replies; 15+ messages in thread
From: Eliot Courtney @ 2026-08-13 7:29 UTC (permalink / raw)
To: Yury Norov, Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
dri-devel
On Thu Aug 13, 2026 at 6:16 AM JST, Yury Norov wrote:
> On Wed, Aug 12, 2026 at 05:51:24PM +0900, Eliot Courtney wrote:
>> Add support for contiguous area allocation. Add a new type,
>> `UnusedArea`, following the same pattern as `UnusedId`.
>>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>> rust/kernel/id_pool.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 69 insertions(+)
>>
>> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
>> index 384753fe0e44..eb911a0e3217 100644
>> --- a/rust/kernel/id_pool.rs
>> +++ b/rust/kernel/id_pool.rs
>> @@ -4,8 +4,14 @@
>>
>> //! Rust API for an ID pool backed by a [`BitmapVec`].
>>
>> +use core::{
>> + num::NonZero,
>> + ops::Range, //
>> +};
>> +
>> use crate::alloc::{AllocError, Flags};
>> use crate::bitmap::BitmapVec;
>> +use crate::ptr::Alignment;
>>
>> /// Represents a dynamic ID pool backed by a [`BitmapVec`].
>> ///
>> @@ -240,6 +246,33 @@ pub fn find_unused_id(&mut self, offset: usize) -> Option<UnusedId<'_>> {
>> pub fn release_id(&mut self, id: usize) {
>> self.map.clear_bit(id);
>> }
>> +
>> + /// Finds a contiguous area of `count` unused IDs at or after `offset`.
>> + ///
>> + /// The start of the returned area is a multiple of `align`.
>> + ///
>> + /// Returns an [`UnusedArea`] upon success, or [`None`] if no such area could be found.
>> + #[inline]
>> + #[must_use]
>> + pub fn find_unused_area(
>> + &mut self,
>> + offset: usize,
>> + count: NonZero<usize>,
>> + align: Alignment,
>> + ) -> Option<UnusedArea<'_>> {
>> + let start = self.map.next_zero_area(offset, count.get(), align)?;
>> + // INVARIANT: `next_zero_area()` returns None or a start with `start + count <= map.len()`.
>> + Some(UnusedArea {
>> + range: start..start + count.get(),
>> + pool: self,
>> + })
>> + }
>> +
>> + /// Releases a contiguous area of IDs.
>> + #[inline]
>> + pub fn release_area(&mut self, range: &Range<usize>) {
>> + self.map.clear(range.start, range.len());
>> + }
>> }
>>
>> /// Represents an unused id in an [`IdPool`].
>> @@ -287,6 +320,42 @@ pub fn acquire(self) -> usize {
>> }
>> }
>>
>> +/// Represents an unused, contiguous area of IDs in an [`IdPool`].
>> +///
>> +/// # Invariants
>> +///
>> +/// `range.start <= range.end <= pool.map.len()`.
>> +#[must_use = "the ID range is not reserved unless acquired"]
>> +pub struct UnusedArea<'pool> {
>> + range: Range<usize>,
>> + pool: &'pool mut IdPool,
>> +}
>
> So, the compilation message refers the "ID range", not the UnusedArea.
> To me, this 'unused' language is confusing. What should I do with the
> area that I just allocated? Drop the 'unused' one and create the 'used'?
>
> Can you rename it to id_range please? Then the API would look more
> consistent, at least to me.
tl;dr: I will remove `UnusedArea` according to your suggestion
`UnusedArea` here mirrors the design of `UnusedId`, which first finds
the unused ID then lets you actually set it whenever you want (first
introduced in f523d110a63b ("rust: id_pool: do not immediately acquire
new ids")). According to f523d110a63b, the reason for this design is to
allow some fallible operations once you know the ID you are getting
before committing to actually allocating it. So `UnusedArea` just
follows the existing design for this API. But, we don't need this
intermediate fallible operation behaviour right now, so I think it's ok
to get rid of it.
>
>> +
>> +impl<'pool> UnusedArea<'pool> {
>> + /// Returns the unused ID range.
>> + ///
>> + /// Be aware that the area has not yet been acquired in the pool. The
>> + /// [`acquire`] method must be called to prevent others from taking it.
>> + ///
>> + /// [`acquire`]: UnusedArea::acquire()
>
> So maybe implement the find_acquire() method? In the caller you
> serialize it with:
>
> let mut ids = self.inner.lock();
>
> Is it possible to pass this down to the suggested find_acquire()? In
> my experience, having non-atomic sequence of find + acquire that
> requires the external locking is the recipe for troubles.
In this case, since we stash a mutable reference to `IdPool` in
`UnusedArea` (just like `UnusedId`), it's not possible to hit any
non-atomic find/acquire issues. The compiler will statically prevent you
from being able to allocate anything else in the `IdPool` as long as the
`UnusedArea` is alive. That is, it's always valid to acquire an
`UnusedArea` (or `UnusedId`). In this case IMO it is more flexible to
let the caller decide how it will do the locking (and that's the
existing design for allocating single IDs here), since it can e.g.
control the granularity. Or maybe it really exists in a single threaded
context and doesn't need locking whatsoever (i.e. can get a mutable ref
to IdPool without locking).
>
>> + #[inline]
>> + #[must_use]
>> + pub fn range(&self) -> Range<usize> {
>> + self.range.clone()
>> + }
>> +
>> + /// Acquires the area.
>> + ///
>> + /// Returns the now-reserved ID range.
>> + #[inline]
>> + pub fn acquire(self) -> Range<usize> {
>> + let Self { range, pool } = self;
>> + // By the type invariants, the range is within bounds.
>> + pool.map.set(range.start, range.end - range.start);
>> + range
>
> From hierarchy perspective, the UnusedArea wraps the Range, and
> passing the Range to the higher layer breaks the hierarchy. If you
> follow my suggestion, the hierarchy will be enforced stricter:
>
> ChannelIdRange -> IdRange-> Range
>
> instead of
>
> ChannelIdArea -> UnusedArea-> Range
> |
> -> Range
>
> Or I misunderstand the concept of the UnusedArea?
>
>> + }
>> +}
>> +
>> impl Default for IdPool {
>> #[inline]
>> fn default() -> Self {
>>
>> --
>> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool
2026-08-12 22:18 ` Yury Norov
@ 2026-08-13 7:31 ` Eliot Courtney
2026-08-13 18:32 ` Yury Norov
0 siblings, 1 reply; 15+ messages in thread
From: Eliot Courtney @ 2026-08-13 7:31 UTC (permalink / raw)
To: Yury Norov, Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
dri-devel
On Thu Aug 13, 2026 at 7:18 AM JST, Yury Norov wrote:
> On Wed, Aug 12, 2026 at 05:51:25PM +0900, Eliot Courtney wrote:
>> Add `ChannelIdPool` which adds automatic tracking and releasing of
>> channel IDs on top of `IdPool`. This is necessary for apportioning
>> ranges of channel IDs to be used in e.g. vGPU.
>>
>> Channel IDs are allocated as a contiguous sequence with a specific
>> length and sometimes a specific alignment [1] for vGPU. The ID space is
>> small (limited to 2048) and allocation is not on a hot path, so a
>> bitmap-backed `IdPool` is a better fit than IDA/xarray (which allocate a
>> single ID within a range, not a contiguous sequence) or a maple tree
>> (where aligned allocation needs an alloc_range()+erase() retry loop that
>> essentially reimplements bitmap_find_next_zero_area()) [2]. It is
>> also faster than maple tree [3].
>>
>> Link: https://lore.kernel.org/all/84bc8bd2-e292-4b84-9580-a1b5df4c5bdc@nvidia.com/ # [1]
>> Link: https://lore.kernel.org/all/20260710-chid-maple-v1-1-4ee869055268@nvidia.com/ # [2]
>> Link: https://lore.kernel.org/all/20260717053241.916441-1-ynorov@nvidia.com/ # [3]
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>> drivers/gpu/nova-core/gpu.rs | 2 +
>> drivers/gpu/nova-core/gpu/channel.rs | 180 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 182 insertions(+)
>>
>> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
>> index 42a4cd7971fa..66ea697a89f8 100644
>> --- a/drivers/gpu/nova-core/gpu.rs
>> +++ b/drivers/gpu/nova-core/gpu.rs
>> @@ -33,6 +33,8 @@
>> vgpu::VgpuManager, //
>> };
>>
>> +#[cfg_attr(not(CONFIG_KUNIT = "y"), expect(dead_code))]
>> +mod channel;
>> mod hal;
>>
>> macro_rules! define_chipset {
>> diff --git a/drivers/gpu/nova-core/gpu/channel.rs b/drivers/gpu/nova-core/gpu/channel.rs
>> new file mode 100644
>> index 000000000000..b755d2184aee
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/gpu/channel.rs
>> @@ -0,0 +1,180 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
>> +
>> +//! Channel ID allocation.
>> +
>> +use core::{
>> + num::NonZero,
>> + ops::{
>> + Deref,
>> + Range, //
>> + }, //
>> +};
>> +
>> +use kernel::{
>> + id_pool::IdPool,
>> + prelude::*,
>> + ptr::Alignment,
>> + sync::{
>> + new_mutex,
>> + Mutex, //
>> + }, //
>> +};
>> +
>> +/// Pool for tracking reservations of channel IDs.
>> +#[pin_data]
>> +pub(crate) struct ChannelIdPool {
>> + #[pin]
>> + inner: Mutex<IdPool>,
>> + num_chids: usize,
>> +}
>> +
>> +impl ChannelIdPool {
>> + /// Creates a pool managing `num_chids` channel IDs.
>> + pub(crate) fn new(num_chids: usize) -> impl PinInit<Self, Error> {
>> + try_pin_init!(Self {
>> + inner <- new_mutex!(IdPool::with_capacity(num_chids, GFP_KERNEL)?),
>> + num_chids,
>> + })
>> + }
>> +
>> + /// Reserves a contiguous area of `count` channel IDs starting at a multiple of `align`,
>> + /// returning a guard that releases the area on drop.
>> + pub(crate) fn alloc_area(
>> + &self,
>> + count: NonZero<usize>,
>
> OK, here you use NonZero. Please do that in the lowest layer.
Will do~
>
>> + align: Alignment,
>> + ) -> Result<ChannelIdArea<'_>> {
>> + let mut ids = self.inner.lock();
>> + let area = ids.find_unused_area(0, count, align).ok_or(ENOSPC)?;
>> +
>> + // If the pool is small, the backing bitmap may be rounded up to a larger size.
>
> Not sure I understand this language. Your ID pool is a fixed-size. Or
> do you mean something else?
Yeah this is a little confusing. The reason this exists is because
`IdPool::with_capacity` will round up the size to
`BitmapVec::MAX_INLINE_LEN`. For binder, there isn't a natural limit to
the ID space IIUC so it's not a problem there.
Anyway, in this case, IdPool can return an area that is outside of the
original capacity you gave to `IdPool::with_capacity`, hence this check.
But, I had a closer look at IdPool::with_capacity, and I can't see a
good reason for why it's doing this adjusting to a min of
`BitmapVec::MAX_INLINE_LEN`. So let me try to instead remove that
behaviour.
>
>> + if area.range().end > self.num_chids {
>> + return Err(ENOSPC);
>> + }
>> + Ok(ChannelIdArea {
>> + pool: self,
>> + range: area.acquire(),
>> + })
>> + }
>> +}
>> +
>> +/// A reserved contiguous area of channel IDs.
>> +///
>> +/// Releases the whole area back to its [`ChannelIdPool`] when dropped. Releasing locks a
>> +/// sleeping [`Mutex`], so the area must be dropped in a context that is allowed to sleep.
>> +#[must_use = "the channel ID area is released immediately when unused"]
>> +pub(crate) struct ChannelIdArea<'a> {
>> + pool: &'a ChannelIdPool,
>> + range: Range<usize>,
>> +}
>> +
>> +impl Drop for ChannelIdArea<'_> {
>> + fn drop(&mut self) {
>> + self.pool.inner.lock().release_area(&self.range);
>> + }
>> +}
>> +
>> +impl Deref for ChannelIdArea<'_> {
>> + type Target = Range<usize>;
>> +
>> + fn deref(&self) -> &Self::Target {
>> + &self.range
>> + }
>> +}
>> +
>> +#[kunit_tests(nova_core_channel)]
>> +mod tests {
>> + use super::*;
>> +
>> + const fn nz<const N: usize>() -> NonZero<usize> {
>> + const { NonZero::new(N).unwrap() }
>> + }
>> +
>> + #[test]
>> + fn chid_area() -> Result {
>> + let pool = KBox::pin_init(ChannelIdPool::new(2048), GFP_KERNEL)?;
>> + let unaligned = Alignment::new::<1>();
>> +
>> + let first = pool.alloc_area(nz::<48>(), unaligned)?;
>> + assert_eq!(0, first.start);
>> + assert_eq!(48, first.len());
>> + assert_eq!(48, first.end);
>> +
>> + let second = pool.alloc_area(nz::<48>(), unaligned)?;
>> + assert!(first.end <= second.start || second.end <= first.start);
>> +
>> + let first_start = first.start;
>> + drop(first);
>
> You test the drop() only once. Can you add more tests? At least, make
> sure that 2 allocs followed by 2 drops ends up with an empty pool.
Yerp good idea. Thanks!
>
>> + assert_eq!(first_start, pool.alloc_area(nz::<48>(), unaligned)?.start);
>> + Ok(())
>> + }
>> +
>> + #[test]
>> + fn chid_bounded_by_num_chids() -> Result {
>> + let pool = KBox::pin_init(ChannelIdPool::new(4), GFP_KERNEL)?;
>> + let unaligned = Alignment::new::<1>();
>> +
>> + {
>> + let a = pool.alloc_area(nz::<1>(), unaligned)?;
>> + let b = pool.alloc_area(nz::<1>(), unaligned)?;
>> + let c = pool.alloc_area(nz::<1>(), unaligned)?;
>> + let d = pool.alloc_area(nz::<1>(), unaligned)?;
>
> OK, here your alloc_area() means the find + alloc, and it returns
> a Range - not area.
>
> To me it looks like the intermediate UnusedArea layer is excessive.
> If you just do find + alloc in this pool.alloc_area(), you seemingly
> don't need the UnusedArea.
>
> Can you try without it, please?
Yes, you're correct that `UnusedArea` layer isn't strictly required. We
are using it once to handle the case when IdPool gives us back something
that is outside of what we originally requested. But we could also
handle this just by bitmap setting the range
`num_chids..pool.capacity()` on creation of `ChannelIdPool`, runtime
checking num_chids >= BitmapVec::MAX_INLINE_LEN, or changing IdPool to
always create a bitmap of the specified capacity (this is what I'll do
unless someone knows a good reason not to).
>
>> + assert_eq!(0, a.start);
>> + assert_eq!(1, b.start);
>> + assert_eq!(2, c.start);
>> + assert_eq!(3, d.start);
>> + assert_eq!(
>> + Err(ENOSPC),
>> + pool.alloc_area(nz::<1>(), unaligned).map(|_| ())
>> + );
>> + }
>> +
>> + assert_eq!(0, pool.alloc_area(nz::<4>(), unaligned)?.start);
>> + assert_eq!(
>> + Err(ENOSPC),
>> + pool.alloc_area(nz::<5>(), unaligned).map(|_| ())
>> + );
>> +
>> + let head = pool.alloc_area(nz::<3>(), unaligned)?;
>> + assert_eq!(0, head.start);
>> + assert_eq!(
>> + Err(ENOSPC),
>> + pool.alloc_area(nz::<2>(), unaligned).map(|_| ())
>> + );
>> + assert_eq!(3, pool.alloc_area(nz::<1>(), unaligned)?.start);
>> + Ok(())
>> + }
>> +
>> + #[test]
>> + fn chid_area_aligned() -> Result {
>> + let pool = KBox::pin_init(ChannelIdPool::new(16), GFP_KERNEL)?;
>> + let unaligned = Alignment::new::<1>();
>> + let align4 = Alignment::new::<4>();
>> +
>> + // Alloc 0 so the first fit for the next area is unaligned.
>> + let pad = pool.alloc_area(nz::<1>(), unaligned)?;
>> + assert_eq!(0, pad.start);
>> +
>> + let a = pool.alloc_area(nz::<4>(), align4)?;
>> + assert_eq!(4, a.start);
>> +
>> + // The area skipped over by the aligned allocation should still be available.
>> + let b = pool.alloc_area(nz::<1>(), unaligned)?;
>> + assert_eq!(1, b.start);
>> +
>> + let c = pool.alloc_area(nz::<8>(), Alignment::new::<8>())?;
>
> Is it possible to make it somehow simpler:
>
> let c = pool.alloc_area(8, 8)?;
>
> All the parameters checking must be a part of implementations, not the
> interface.
>
> We had a very similar discussion in the bitfields implementation thread,
> and many people in CC list of this thread spent quite a long time to find
> a way from:
>
> let color = Rgb::default()
> .set_red(Bounded::<u16, _>::new::<0x10>())
> .set_green(Bounded::<u16, _>::new::<0x1f>())
> .set_blue(Bounded::<u16, _>::new::<0x18>());
>
> to:
>
>
> let color = Rgb::default().
> .set_red(0x10)
> .set_green(0x1f)
> .set_blue(0x18)
>
> Can you do the same here? Please refer:
>
> https://lore.kernel.org/all/aXCZeVqkDrBWr1uq@yury/
I think that taking NonZero and Alignment here obviates the need for
checking the parameters, since they have their own guarantees (and Alice
recommended using Alignment on `Bitmap` too for this reason IIUC). Maybe
I am misundertanding but we spent a few iterations here adding
`Alignment` and `NonZero` on various parameters -- do you mean just
making ChannelIdPool::alloc_area work with a plain integer syntax? It's
possible to just take plain integers here and check, but I don't think
it's necessarily better.
W.r.t. the bitfield stuff, yeah I agree that was a good call since that
syntax was very verbose, and IIUC that was resolved by having e.g.
with_const_red::<0x10>(). The analogous change here would be to provide
const generic args, e.g. alloc_area_const::<size, align>() which could
be plain integers. But, in practice the arguments to alloc_area are
going to be runtime values (outside of tests) that the caller has
strictly more info about. Having the separate types (NonZero, Alignment)
also makes easier to not mix up the order. I can't think of a way to
remove this verbosity without just passing plain integer runtime values,
which IMO is not great.
>
>> + assert_eq!(8, c.start);
>> +
>> + // Only 2 IDs left.
>> + assert_eq!(Err(ENOSPC), pool.alloc_area(nz::<4>(), align4).map(|_| ()));
>> + assert_eq!(
>> + Err(ENOSPC),
>> + pool.alloc_area(nz::<1>(), Alignment::new::<32>())
>> + .map(|_| ())
>> + );
>> +
>> + assert_eq!(2, pool.alloc_area(nz::<2>(), unaligned)?.start);
>> + Ok(())
>> + }
>> +}
>>
>> --
>> 2.55.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool
2026-08-13 7:31 ` Eliot Courtney
@ 2026-08-13 18:32 ` Yury Norov
0 siblings, 0 replies; 15+ messages in thread
From: Yury Norov @ 2026-08-13 18:32 UTC (permalink / raw)
To: Eliot Courtney
Cc: Alice Ryhl, Burak Emir, Yury Norov, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter,
Greg Kroah-Hartman, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, rust-for-linux, linux-kernel, nova-gpu, dri-devel,
dri-devel
On Thu, Aug 13, 2026 at 04:31:26PM +0900, Eliot Courtney wrote:
> On Thu Aug 13, 2026 at 7:18 AM JST, Yury Norov wrote:
> > On Wed, Aug 12, 2026 at 05:51:25PM +0900, Eliot Courtney wrote:
...
> >> + let c = pool.alloc_area(nz::<8>(), Alignment::new::<8>())?;
> >
> > Is it possible to make it somehow simpler:
> >
> > let c = pool.alloc_area(8, 8)?;
> >
> > All the parameters checking must be a part of implementations, not the
> > interface.
> >
> > We had a very similar discussion in the bitfields implementation thread,
> > and many people in CC list of this thread spent quite a long time to find
> > a way from:
> >
> > let color = Rgb::default()
> > .set_red(Bounded::<u16, _>::new::<0x10>())
> > .set_green(Bounded::<u16, _>::new::<0x1f>())
> > .set_blue(Bounded::<u16, _>::new::<0x18>());
> >
> > to:
> >
> >
> > let color = Rgb::default().
> > .set_red(0x10)
> > .set_green(0x1f)
> > .set_blue(0x18)
> >
> > Can you do the same here? Please refer:
> >
> > https://lore.kernel.org/all/aXCZeVqkDrBWr1uq@yury/
>
> I think that taking NonZero and Alignment here obviates the need for
> checking the parameters, since they have their own guarantees (and Alice
> recommended using Alignment on `Bitmap` too for this reason IIUC). Maybe
> I am misundertanding but we spent a few iterations here adding
> `Alignment` and `NonZero` on various parameters -- do you mean just
> making ChannelIdPool::alloc_area work with a plain integer syntax? It's
> possible to just take plain integers here and check, but I don't think
> it's necessarily better.
>
> W.r.t. the bitfield stuff, yeah I agree that was a good call since that
> syntax was very verbose, and IIUC that was resolved by having e.g.
> with_const_red::<0x10>(). The analogous change here would be to provide
> const generic args, e.g. alloc_area_const::<size, align>() which could
> be plain integers. But, in practice the arguments to alloc_area are
> going to be runtime values (outside of tests) that the caller has
> strictly more info about. Having the separate types (NonZero, Alignment)
> also makes easier to not mix up the order. I can't think of a way to
> remove this verbosity without just passing plain integer runtime values,
> which IMO is not great.
pub(crate) fn alloc_area(
&self,
count: usize,
align: usize,
) -> Result<ChannelIdArea<'_>> {
let count = NonZero::new(count).ok_or(EINVAL)?;
let align = Alignment::new_checked(align).ok_or(EINVAL)?;
let mut ids = self.inner.lock();
let area = ids.find_unused_area(0, count, align).ok_or(ENOSPC)?;
// If the pool is small, the backing bitmap may be rounded up to a larger size.
if area.range().end > self.num_chids {
return Err(ENOSPC);
}
Ok(ChannelIdArea {
pool: self,
range: area.acquire(),
})
}
let area = pool.alloc_area(8, 4)?;
See the difference? You still check the parameters, but don't make it
the part of interface.
And from practical perspective, your users simply call the function,
not tinkering around your 'safety measures'.
In the next version, if you drop the intermediate UnusedArea layer,
you may want to do a C-like check instead of creating new types,
because here you'll directly call C function. And it's completely OK.
Not OK is complicating interfaces and life of your users.
Thanks,
Yury
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-08-13 18:32 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-12 22:23 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-08-12 19:44 ` Yury Norov
2026-08-12 8:51 ` [PATCH v5 3/5] rust: bitmap: add contiguous area operations Eliot Courtney
2026-08-12 20:31 ` Yury Norov
2026-08-13 7:27 ` Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 4/5] rust: id_pool: add contiguous area allocation Eliot Courtney
2026-08-12 21:16 ` Yury Norov
2026-08-13 7:29 ` Eliot Courtney
2026-08-12 8:51 ` [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-08-12 22:18 ` Yury Norov
2026-08-13 7:31 ` Eliot Courtney
2026-08-13 18:32 ` Yury Norov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox