linux-kernel - Re: [PATCH v3 3/5] rust: id_pool: do not supply starting capacity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DDWYFIZLT334.1R58VVG8U59T7@nvidia.com>
Date: Sat, 01 Nov 2025 10:07:42 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Alice Ryhl" <aliceryhl@...gle.com>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Yury Norov" <yury.norov@...il.com>
Cc: Arve Hjønnevåg <arve@...roid.com>, "Todd Kjos"
 <tkjos@...roid.com>, "Martijn Coenen" <maco@...roid.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Christian Brauner" <brauner@...nel.org>, "Carlos
 Llamas" <cmllamas@...gle.com>, "Suren Baghdasaryan" <surenb@...gle.com>,
 "Burak Emir" <bqe@...gle.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Trevor
 Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/5] rust: id_pool: do not supply starting capacity

On Tue Oct 28, 2025 at 7:55 PM JST, Alice Ryhl wrote:
> Rust Binder wants to use inline bitmaps whenever possible to avoid
> allocations, so introduce a constructor for an IdPool with arbitrary
> capacity that stores the bitmap inline.
>
> The existing constructor could be renamed to with_capacity() to match
> constructors for other similar types, but it is removed as there is
> currently no user for it.
>
> Acked-by: Yury Norov (NVIDIA) <yury.norov@...il.com>
> Reviewed-by: Burak Emir <bqe@...gle.com>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
>  rust/kernel/id_pool.rs | 46 ++++++++++++++++++----------------------------
>  1 file changed, 18 insertions(+), 28 deletions(-)
>
> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index a41a3404213ca92d53b14c80101afff6ac8c416e..d53628a357ed84a6e00ef9dfd03a75e85a87532c 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -28,19 +28,21 @@
>  /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
>  /// use kernel::id_pool::IdPool;
>  ///
> -/// let mut pool = IdPool::new(64, GFP_KERNEL)?;
> -/// for i in 0..64 {
> +/// let mut pool = IdPool::new();
> +/// let cap = pool.capacity();
> +///
> +/// for i in 0..cap {
>  ///     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
>  /// }
>  ///
> -/// pool.release_id(23);
> -/// assert_eq!(23, pool.acquire_next_id(0).ok_or(ENOSPC)?);
> +/// pool.release_id(5);
> +/// assert_eq!(5, pool.acquire_next_id(0).ok_or(ENOSPC)?);
>  ///
>  /// assert_eq!(None, pool.acquire_next_id(0));  // time to realloc.
>  /// let resizer = pool.grow_request().ok_or(ENOSPC)?.realloc(GFP_KERNEL)?;
>  /// pool.grow(resizer);
>  ///
> -/// assert_eq!(pool.acquire_next_id(0), Some(64));
> +/// assert_eq!(pool.acquire_next_id(0), Some(cap));
>  /// # Ok::<(), Error>(())
>  /// ```
>  ///
> @@ -96,16 +98,11 @@ pub fn realloc(&self, flags: Flags) -> Result<PoolResizer, AllocError> {
>  
>  impl IdPool {
>      /// Constructs a new [`IdPool`].
> -    ///
> -    /// A capacity below [`BITS_PER_LONG`] is adjusted to
> -    /// [`BITS_PER_LONG`].
> -    ///
> -    /// [`BITS_PER_LONG`]: srctree/include/asm-generic/bitsperlong.h

A word about the capacity of this pool would be helpful here IMHO -
without anything one could assume it is 0, which is not the case.

>      #[inline]
> -    pub fn new(num_ids: usize, flags: Flags) -> Result<Self, AllocError> {
> -        let num_ids = core::cmp::max(num_ids, BITS_PER_LONG);
> -        let map = BitmapVec::new(num_ids, flags)?;
> -        Ok(Self { map })
> +    pub fn new() -> Self {
> +        Self {
> +            map: BitmapVec::new_inline(),
> +        }
>      }
>  
>      /// Returns how many IDs this pool can currently have.
> @@ -119,20 +116,6 @@ pub fn capacity(&self) -> usize {
>      /// The capacity of an [`IdPool`] cannot be shrunk below [`BITS_PER_LONG`].
>      ///
>      /// [`BITS_PER_LONG`]: srctree/include/asm-generic/bitsperlong.h
> -    ///
> -    /// # Examples
> -    ///
> -    /// ```
> -    /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
> -    /// use kernel::id_pool::{ReallocRequest, IdPool};
> -    ///
> -    /// let mut pool = IdPool::new(1024, GFP_KERNEL)?;
> -    /// let alloc_request = pool.shrink_request().ok_or(AllocError)?;
> -    /// let resizer = alloc_request.realloc(GFP_KERNEL)?;
> -    /// pool.shrink(resizer);
> -    /// assert_eq!(pool.capacity(), kernel::bindings::BITS_PER_LONG as usize);
> -    /// # Ok::<(), AllocError>(())

Is this test deleted because the old constructor has been removed? If so
I'd say that justifies keeping it around, having tests is valuable in
itself and the example is quite helpful to understand how the
not-so-obvious resizing process works.