On Sat, Jul 11, 2026 at 12:46:40PM +0200, Christian König wrote:
> On 7/11/26 04:56, Matthew Brost wrote:
> > Populating a GTT (system) buffer object under the reservation lock can
> > stall in reclaim and compaction while trying to satisfy beneficial-order
> > allocations, holding the lock for the duration.
> 
> Yeah and that is perfectly intentional behavior.
> 
> I really don't see any reason for any driver to change that.
> 
> The purpose of the dma_resv lock is to stall other allocation for the same 
> object it protects, making sure that we don't have multiple threads 
> allocating memory for the same buffer object.
> 
> Why in the world should that be a problem?
> 

See my example in my reply to the cover letter: multiple threads
sharing a VM, with a memory allocation performed under the VM dma-resv
lock, which in turn blocks exec IOCTLs, CPU page faults or VM binds on
BOs sharing the same VM dma-resv lock. With that, this code could be
gating on whether the BO allocation shares a VM dma-resv lock.

The most relevant example here, I believe, is that launching a new
Chrome tab could potentially stall the aforementioned operations in a
different Chrome tab. Likewise, if games allocate memory on demand
(they typically do not, because memory allocations are prone to
introducing stalls), they could also incur the aforementioned stalls.

I think the opposite question is more appropriate here: does
preallocating memory outside of dma-resv before a device can DMA to it
break any invariants? The answer is no; it does not in the case of GEM
create IOCTLs or the defragmentation moves introduced in this series.

Matt

> Regards,
> Christian.
> 
> > 
> > Mirror the Xe out-of-lock preallocation for pure system BOs: when a
> > gem_create request targets AMDGPU_GEM_DOMAIN_GTT only, fill the full page
> > backing up front via ttm_pool_prealloc_fill_full() before taking the
> > reservation lock. The populate under the lock then simply installs these
> > pages instead of reclaiming and compacting in the critical section. The
> > fill is best-effort - a short fill falls back to the normal in-lock
> > allocation for the remaining pages, and any leftover pages are released
> > by ttm_pool_prealloc_fini() on all exit paths.
> > 
> > Unlike Xe, amdgpu has no background defragmenter, so the higher-order
> > reclaim backoff is left disabled (normal reclaim). If the pool uses
> > dma-alloc (swiotlb), the fill bails and the feature is a silent no-op.
> > 
> > Factor the tt pool selection into amdgpu_ttm_tt_pool_id() and
> > amdgpu_ttm_pool() so the preallocation targets the exact pool the
> > populate will consume, and thread an optional prealloc bag through
> > amdgpu_gem_object_create() and amdgpu_bo_param into the populate ctx.
> > 
> > Cc: [email protected]
> > Cc: Alex Deucher <[email protected]>
> > Cc: Carlos Santa <[email protected]>
> > Cc: Ryan Neph <[email protected]>
> > Cc: Christian Koenig <[email protected]>
> > Cc: Huang Rui <[email protected]>
> > Cc: Matthew Auld <[email protected]>
> > Cc: Maarten Lankhorst <[email protected]>
> > Cc: Maxime Ripard <[email protected]>
> > Cc: Thomas Zimmermann <[email protected]>
> > Cc: David Airlie <[email protected]>
> > Cc: Simona Vetter <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: Thomas Hellström <[email protected]>
> > Assisted-by: GitHub_Copilot:claude-opus-4.8
> > Signed-off-by: Matthew Brost <[email protected]>
> > 
> > ---
> > 
> > v3:
> >  - Keep WC caching only when USWC is supported and bound the
> >    preallocation to available GTT space (Sashiko)
> > ---
> >  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  4 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       | 73 +++++++++++++++++--
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h       |  5 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h    |  4 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 39 +++++++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h       |  3 +
> >  8 files changed, 108 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 20831dbebc31..940f58848a97 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -345,7 +345,7 @@ create_dmamap_sg_bo(struct amdgpu_device *adev,
> >  
> >     ret = amdgpu_gem_object_create(adev, mem->bo->tbo.base.size, 1,
> >                     AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_CREATE_PREEMPTIBLE | 
> > flags,
> > -                   ttm_bo_type_sg, mem->bo->tbo.base.resv, &gem_obj, 0);
> > +                   ttm_bo_type_sg, mem->bo->tbo.base.resv, &gem_obj, 0, 
> > NULL);
> >  
> >     amdgpu_bo_unreserve(mem->bo);
> >  
> > @@ -1811,7 +1811,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> >              domain_string(alloc_domain), xcp_id);
> >  
> >     ret = amdgpu_gem_object_create(adev, aligned_size, 1, alloc_domain, 
> > alloc_flags,
> > -                                  bo_type, NULL, &gobj, xcp_id + 1);
> > +                                  bo_type, NULL, &gobj, xcp_id + 1, NULL);
> >     if (ret) {
> >             pr_debug("Failed to create BO on domain %s. ret %d\n",
> >                      domain_string(alloc_domain), ret);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > index b33c300e26e2..51510e831129 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > @@ -435,7 +435,7 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, 
> > struct dma_buf *dma_buf)
> >  
> >     ret = amdgpu_gem_object_create(adev, dma_buf->size, PAGE_SIZE,
> >                                    AMDGPU_GEM_DOMAIN_CPU, flags,
> > -                                  ttm_bo_type_sg, resv, &gobj, 0);
> > +                                  ttm_bo_type_sg, resv, &gobj, 0, NULL);
> >     if (ret)
> >             goto error;
> >  
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > index 6a0699746fbc..e8b732218e7d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -39,6 +39,7 @@
> >  #include <drm/drm_exec.h>
> >  #include <drm/drm_gem_ttm_helper.h>
> >  #include <drm/ttm/ttm_tt.h>
> > +#include <drm/ttm/ttm_pool.h>
> >  #include <drm/drm_syncobj.h>
> >  
> >  #include "amdgpu.h"
> > @@ -168,7 +169,8 @@ int amdgpu_gem_object_create(struct amdgpu_device 
> > *adev, unsigned long size,
> >                          int alignment, u32 initial_domain,
> >                          u64 flags, enum ttm_bo_type type,
> >                          struct dma_resv *resv,
> > -                        struct drm_gem_object **obj, int8_t xcp_id_plus1)
> > +                        struct drm_gem_object **obj, int8_t xcp_id_plus1,
> > +                        struct ttm_pool_prealloc *prealloc)
> >  {
> >     struct amdgpu_bo *bo;
> >     struct amdgpu_bo_user *ubo;
> > @@ -188,6 +190,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
> > *adev, unsigned long size,
> >     bp.domain = initial_domain;
> >     bp.bo_ptr_size = sizeof(struct amdgpu_bo);
> >     bp.xcp_id_plus1 = xcp_id_plus1;
> > +   bp.prealloc = prealloc;
> >  
> >     r = amdgpu_bo_create_user(adev, &bp, &ubo);
> >     if (r)
> > @@ -412,6 +415,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, 
> > void *data,
> >     struct dma_resv *resv = NULL;
> >     struct drm_gem_object *gobj;
> >     uint32_t handle, initial_domain;
> > +   struct ttm_pool_prealloc prealloc = {};
> > +   struct ttm_pool *prealloc_pool = NULL;
> >     int r;
> >  
> >     /* reject invalid gem flags */
> > @@ -443,10 +448,57 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, 
> > void *data,
> >             flags |= AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> >     }
> >  
> > +   /*
> > +    * For system-only (pure GTT) BOs, preallocate the whole page backing
> > +    * up front, outside the reservation lock. Populate under the lock then
> > +    * just installs these pages instead of reclaiming/compacting in the
> > +    * critical section. Best-effort: a short fill falls back to the normal
> > +    * in-lock allocation for the missing pages.
> > +    */
> > +   if (args->in.domains == AMDGPU_GEM_DOMAIN_GTT) {
> > +           struct ttm_resource_manager *gtt_man =
> > +                   ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
> > +           int32_t xcp_id = adev->gmc.mem_partitions ? fpriv->xcp_id : 0;
> > +           int32_t pool_id = amdgpu_ttm_tt_pool_id(adev, xcp_id);
> > +           /*
> > +            * Mirror the USWC handling in amdgpu_bo_create(): the flag is
> > +            * stripped when the platform can't do write-combining, in which
> > +            * case the tt is created ttm_cached. The prealloc caching must
> > +            * match the final tt caching exactly, otherwise ttm_pool_free()
> > +            * would restore the wrong PAT state on these pages (PAT 
> > aliasing
> > +            * / leak of WC pages into the cached allocator).
> > +            */
> > +           bool uswc = (flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC) &&
> > +                   amdgpu_bo_support_uswc(flags);
> > +           enum ttm_caching caching =
> > +                   uswc ? ttm_write_combined : ttm_cached;
> > +
> > +           /*
> > +            * Only prealloc when the request fits the currently-available
> > +            * GTT (total manager size minus current usage). This mirrors 
> > the
> > +            * amdgpu_bo_validate_size() bound applied later in
> > +            * amdgpu_bo_create() while also skipping the up-front reclaim /
> > +            * compaction when GTT is already near full or the 
> > user-controlled
> > +            * size is over-large (the creation path handles those anyway).
> > +            */
> > +           if (gtt_man) {
> > +                   u64 used = ttm_resource_manager_usage(gtt_man);
> > +                   u64 avail = gtt_man->size > used ?
> > +                           gtt_man->size - used : 0;
> > +
> > +                   if (size <= avail) {
> > +                           prealloc_pool = amdgpu_ttm_pool(adev, pool_id);
> > +                           ttm_pool_prealloc_fill_full(prealloc_pool,
> > +                                                       caching, &prealloc,
> > +                                                       PFN_UP(size), 
> > false);
> > +                   }
> > +           }
> > +   }
> > +
> >     if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
> >             r = amdgpu_bo_reserve(vm->root.bo, false);
> >             if (r)
> > -                   return r;
> > +                   goto out_prealloc;
> >  
> >             resv = vm->root.bo->tbo.base.resv;
> >     }
> > @@ -455,7 +507,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, 
> > void *data,
> >  retry:
> >     r = amdgpu_gem_object_create(adev, size, args->in.alignment,
> >                                  initial_domain,
> > -                                flags, ttm_bo_type_device, resv, &gobj, 
> > fpriv->xcp_id + 1);
> > +                                flags, ttm_bo_type_device, resv, &gobj,
> > +                                fpriv->xcp_id + 1, prealloc_pool ? 
> > &prealloc : NULL);
> >     if (r && r != -ERESTARTSYS) {
> >             if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> >                     flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> > @@ -479,17 +532,21 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, 
> > void *data,
> >             amdgpu_bo_unreserve(vm->root.bo);
> >     }
> >     if (r)
> > -           return r;
> > +           goto out_prealloc;
> >  
> >     r = drm_gem_handle_create(filp, gobj, &handle);
> >     /* drop reference from allocate - handle holds it now */
> >     drm_gem_object_put(gobj);
> >     if (r)
> > -           return r;
> > +           goto out_prealloc;
> >  
> >     memset(args, 0, sizeof(*args));
> >     args->out.handle = handle;
> > -   return 0;
> > +
> > +out_prealloc:
> > +   if (prealloc_pool)
> > +           ttm_pool_prealloc_fini(prealloc_pool, &prealloc);
> > +   return r;
> >  }
> >  
> >  int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
> > @@ -528,7 +585,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, 
> > void *data,
> >  
> >     /* create a gem object to contain this object in */
> >     r = amdgpu_gem_object_create(adev, args->size, 0, AMDGPU_GEM_DOMAIN_CPU,
> > -                                0, ttm_bo_type_device, NULL, &gobj, 
> > fpriv->xcp_id + 1);
> > +                                0, ttm_bo_type_device, NULL, &gobj, 
> > fpriv->xcp_id + 1, NULL);
> >     if (r)
> >             return r;
> >  
> > @@ -1298,7 +1355,7 @@ int amdgpu_mode_dumb_create(struct drm_file 
> > *file_priv,
> >     domain = amdgpu_bo_get_preferred_domain(adev,
> >                             amdgpu_display_supported_domains(adev, flags));
> >     r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
> > -                                ttm_bo_type_device, NULL, &gobj, 
> > fpriv->xcp_id + 1);
> > +                                ttm_bo_type_device, NULL, &gobj, 
> > fpriv->xcp_id + 1, NULL);
> >     if (r)
> >             return -ENOMEM;
> >  
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> > index b558336bc4c6..706aeca011f1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> > @@ -35,6 +35,8 @@
> >  
> >  extern const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
> >  
> > +struct ttm_pool_prealloc;
> > +
> >  unsigned long amdgpu_gem_timeout(uint64_t timeout_ns);
> >  
> >  /*
> > @@ -45,7 +47,8 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, 
> > unsigned long size,
> >                          int alignment, u32 initial_domain,
> >                          u64 flags, enum ttm_bo_type type,
> >                          struct dma_resv *resv,
> > -                        struct drm_gem_object **obj, int8_t xcp_id_plus1);
> > +                        struct drm_gem_object **obj, int8_t xcp_id_plus1,
> > +                        struct ttm_pool_prealloc *prealloc);
> >  int amdgpu_mode_dumb_create(struct drm_file *file_priv,
> >                         struct drm_device *dev,
> >                         struct drm_mode_create_dumb *args);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > index f98bfba59a2c..18c4cf3f35a5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > @@ -632,7 +632,8 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
> >             /* We opt to avoid OOM on system pages allocations */
> >             .gfp_retry_mayfail = true,
> >             .allow_res_evict = bp->type != ttm_bo_type_kernel,
> > -           .resv = bp->resv
> > +           .resv = bp->resv,
> > +           .prealloc = bp->prealloc,
> >     };
> >     struct amdgpu_bo *bo;
> >     unsigned long page_align, size = bp->size;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > index ff11a0903499..11f1d403f152 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> > @@ -45,6 +45,8 @@
> >  #define to_amdgpu_bo_user(abo) container_of((abo), struct amdgpu_bo_user, 
> > bo)
> >  #define to_amdgpu_bo_vm(abo) container_of((abo), struct amdgpu_bo_vm, bo)
> >  
> > +struct ttm_pool_prealloc;
> > +
> >  struct amdgpu_bo_param {
> >     unsigned long                   size;
> >     int                             byte_align;
> > @@ -58,6 +60,8 @@ struct amdgpu_bo_param {
> >     void                            (*destroy)(struct ttm_buffer_object 
> > *bo);
> >     /* xcp partition number plus 1, 0 means any partition */
> >     int8_t                          xcp_id_plus1;
> > +   /* optional out-of-lock preallocated backing (system/GTT only) */
> > +   struct ttm_pool_prealloc        *prealloc;
> >  };
> >  
> >  /* bo virtual addresses in a vm */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index b10b0878df37..c9cd4714f515 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1173,6 +1173,30 @@ void amdgpu_ttm_mmio_remap_free_sgt(struct device 
> > *dev,
> >     kfree(sgt);
> >  }
> >  
> > +/*
> > + * amdgpu_ttm_tt_pool_id - compute the ttm pool id backing a given xcp
> > + *
> > + * Mirrors the mapping used when creating the gtt ttm_tt, so callers that 
> > only
> > + * have an xcp id (e.g. an out-of-lock preallocation before the bo exists) 
> > pick
> > + * the same pool the populate will use.
> > + */
> > +int32_t amdgpu_ttm_tt_pool_id(struct amdgpu_device *adev, int32_t xcp_id)
> > +{
> > +   if (adev->gmc.mem_partitions && xcp_id >= 0)
> > +           return KFD_XCP_MEM_ID(adev, xcp_id);
> > +
> > +   return xcp_id;
> > +}
> > +
> > +/* amdgpu_ttm_pool - select the ttm pool for a given pool id */
> > +struct ttm_pool *amdgpu_ttm_pool(struct amdgpu_device *adev, int32_t 
> > pool_id)
> > +{
> > +   if (adev->mman.ttm_pools && pool_id >= 0)
> > +           return &adev->mman.ttm_pools[pool_id];
> > +
> > +   return &adev->mman.bdev.pool;
> > +}
> > +
> >  /**
> >   * amdgpu_ttm_tt_create - Create a ttm_tt object for a given BO
> >   *
> > @@ -1194,10 +1218,7 @@ static struct ttm_tt *amdgpu_ttm_tt_create(struct 
> > ttm_buffer_object *bo,
> >             return NULL;
> >  
> >     gtt->gobj = &bo->base;
> > -   if (adev->gmc.mem_partitions && abo->xcp_id >= 0)
> > -           gtt->pool_id = KFD_XCP_MEM_ID(adev, abo->xcp_id);
> > -   else
> > -           gtt->pool_id = abo->xcp_id;
> > +   gtt->pool_id = amdgpu_ttm_tt_pool_id(adev, abo->xcp_id);
> >  
> >     if (abo->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC)
> >             caching = ttm_write_combined;
> > @@ -1239,10 +1260,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device 
> > *bdev,
> >     if (ttm->page_flags & TTM_TT_FLAG_EXTERNAL)
> >             return 0;
> >  
> > -   if (adev->mman.ttm_pools && gtt->pool_id >= 0)
> > -           pool = &adev->mman.ttm_pools[gtt->pool_id];
> > -   else
> > -           pool = &adev->mman.bdev.pool;
> > +   pool = amdgpu_ttm_pool(adev, gtt->pool_id);
> >     ret = ttm_pool_alloc(pool, ttm, ctx);
> >     if (ret)
> >             return ret;
> > @@ -1284,10 +1302,7 @@ static void amdgpu_ttm_tt_unpopulate(struct 
> > ttm_device *bdev,
> >  
> >     adev = amdgpu_ttm_adev(bdev);
> >  
> > -   if (adev->mman.ttm_pools && gtt->pool_id >= 0)
> > -           pool = &adev->mman.ttm_pools[gtt->pool_id];
> > -   else
> > -           pool = &adev->mman.bdev.pool;
> > +   pool = amdgpu_ttm_pool(adev, gtt->pool_id);
> >  
> >     return ttm_pool_free(pool, ttm);
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > index ff9e2e346609..f90844937b73 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> > @@ -207,6 +207,9 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
> >  void amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
> >  uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t 
> > type);
> >  
> > +int32_t amdgpu_ttm_tt_pool_id(struct amdgpu_device *adev, int32_t xcp_id);
> > +struct ttm_pool *amdgpu_ttm_pool(struct amdgpu_device *adev, int32_t 
> > pool_id);
> > +
> >  #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
> >  int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo,
> >                              struct amdgpu_hmm_range *range);
> 

Reply via email to