> On 1 Sep 2026, at 13:09, Ke Sun via B4 Relay
> <[email protected]> wrote:
>
> From: Alvin Sun <[email protected]>
>
> Add a per-file VM pool using the IdPool for ID allocation
> and an XArray for VM storage.
Can you say a few more words here? You don’t need to over-explain, but I
don’t think this alone says _why_ your change is needed for the community
at large.
>
> Signed-off-by: Alvin Sun <[email protected]>
> ---
> drivers/gpu/drm/tyr/pool.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/tyr/tyr.rs | 1 +
> 2 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/tyr/pool.rs b/drivers/gpu/drm/tyr/pool.rs
> new file mode 100644
> index 0000000000000..1745116930c3e
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/pool.rs
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +//! Per-drm-file VM id pool.
> +
> +use kernel::{
> + id_pool::IdPool,
> + new_mutex,
> + prelude::*,
> + sync::{
> + Arc,
> + ArcBorrow,
> + Mutex, //
> + },
> + xarray::{
> + AllocKind,
> + XArray, //
> + }, //
> +};
> +
> +use crate::vm::Vm;
> +
> +/// Maximum number of VMs per open file. Matches panthor's
> +/// `PANTHOR_MAX_VMS_PER_FILE`.
> +pub(crate) const PANTHOR_MAX_VMS_PER_FILE: u32 = 32;
> +
> +/// Per-open-file pool of VMs.
> +#[pin_data]
> +pub(crate) struct VmPool<'drm> {
I agree with Deborah that this should be in vm.rs itself.
> + #[pin]
> + ids: Mutex<IdPool>,
Can you add a todo here so we use the XArray directly in the future?
> + #[pin]
> + vms: XArray<Arc<Vm<'drm>>>,
> +}
> +
> +impl<'drm> VmPool<'drm> {
> + /// Creates a new [`VmPool`] with capacity for
> [`PANTHOR_MAX_VMS_PER_FILE`] VMs.
> + pub(crate) fn new() -> Result<impl PinInit<Self>> {
> + let ids = IdPool::with_capacity(PANTHOR_MAX_VMS_PER_FILE as usize,
> GFP_KERNEL)?;
> + Ok(pin_init!(Self {
> + ids <- new_mutex!(ids),
> + vms <- XArray::new(AllocKind::Alloc),
> + }))
> + }
> +
> + /// Inserts a VM into the pool, returning the allocated ID.
> + pub(crate) fn add(&self, vm: ArcBorrow<'_, Vm<'drm>>) -> Result<u32> {
> + let id = {
> + let mut ids = self.ids.lock();
> + ids.find_unused_id(1).ok_or(ENOSPC)?.acquire()
> + };
> +
> + let vm: Arc<Vm<'drm>> = vm.into();
> + let mut vms = self.vms.lock();
> + match vms.store(id, vm, GFP_KERNEL) {
> + Ok(previous) => {
> + // Drop the previous entry (expected `None`).
> + drop(previous);
> + Ok(id as u32)
> + }
> + Err(err) => {
> + // Drop the XArray spinlock before acquiring the `ids` mutex.
> + drop(vms);
> + // Release the stored entry and the pooled id.
> + drop(err.value);
> + let mut ids = self.ids.lock();
> + ids.release_id(id);
> + Err(err.error)
> + }
> + }
> + }
> +
> + /// Removes the VM with the given ID.
> + pub(crate) fn remove(&self, id: u32) -> Result<Arc<Vm<'drm>>> {
> + let mut vms = self.vms.lock();
> + match vms.remove(id as usize) {
> + Some(vm) => {
> + drop(vms);
> + let mut ids = self.ids.lock();
> + ids.release_id(id as usize);
> + Ok(vm)
> + }
> + None => Err(EINVAL),
> + }
> + }
> +
> + /// Gets the VM with the given ID.
> + pub(crate) fn get(&self, id: u32) -> Option<Arc<Vm<'drm>>> {
> + let vms = self.vms.lock();
> + let borrow = vms.get(id as usize)?;
> + Some(Arc::from(borrow))
> + }
> +
> + /// Removes and returns the first VM in the pool.
> + pub(crate) fn pop_first(&self) -> Option<Arc<Vm<'drm>>> {
> + for id in 0..PANTHOR_MAX_VMS_PER_FILE {
This should be inclusive, i.e. “..=“ instead of “..”. But a
better solution is to do:
for id in 0..self.ids.lock().capacity() {
..
}
> + if let Ok(vm) = self.remove(id) {
> + return Some(vm);
> + }
> + }
> + None
> + }
> +}
> diff --git a/drivers/gpu/drm/tyr/tyr.rs b/drivers/gpu/drm/tyr/tyr.rs
> index e7ec450bdc9c0..8c0c18c1a970b 100644
> --- a/drivers/gpu/drm/tyr/tyr.rs
> +++ b/drivers/gpu/drm/tyr/tyr.rs
> @@ -13,6 +13,7 @@
> mod gem;
> mod gpu;
> mod mmu;
> +mod pool;
> mod regs;
> mod slot;
> mod vm;
>
> --
> 2.43.0
>
>
