On 2026. szeptember 28., hétfő 11:10:33 keleti államokbeli nyári idő Christian König wrote: > Taking the eviction lock is actually just one step which we need to do > in the critical section handling. > > Rename the functions to reflect that, use the update parameters instead of > the vm to save the GFP flags. > > v2: rebased and reordered > v3: fix rebase artefact, fix error handling in amdgpu_vm_pt_alloc > > Signed-off-by: Christian König <[email protected]> > Reviewed-by: Natalie Vock <[email protected]>
Reviewed-by: Timur Kristóf <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 - > .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h | 43 ++++++++++++++----- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 40 ++++++++--------- > 4 files changed, 54 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 29a66e39f3d60..7b9494375649f > 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1170,11 +1170,9 @@ int amdgpu_vm_update_range(struct amdgpu_device > *adev, struct amdgpu_vm *vm, params.override_pte = allow_override && > adev->gmc.override_pte; > INIT_LIST_HEAD(¶ms.tlb_flush_waitlist); > > - amdgpu_vm_eviction_lock(vm); > - if (vm->evicting) { > - r = -EBUSY; > + r = amdgpu_vm_begin_critical(¶ms); > + if (r) > goto error_free; > - } > > if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) { > struct dma_fence *tmp = dma_fence_get_stub(); > @@ -1258,7 +1256,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, > struct amdgpu_vm *vm, > > error_free: > kfree(tlb_cb); > - amdgpu_vm_eviction_unlock(vm); > + amdgpu_vm_end_critical(¶ms); > drm_dev_exit(idx); > return r; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 0f6634b749746..98cdd7e3475fb > 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > @@ -289,7 +289,6 @@ struct amdgpu_vm { > */ > struct mutex eviction_lock; > bool evicting; > - unsigned int saved_flags; > > /* Memory statistics for this vm, protected by stats_lock */ > spinlock_t stats_lock; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index > ca86eaac75235..8ebb0b033291e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h > @@ -51,7 +51,7 @@ struct amdgpu_vm_update_params { > struct amdgpu_device *adev; > > /** > - * @vm: optional amdgpu_vm we do this update for > + * @vm: amdgpu_vm we do this update for > */ > struct amdgpu_vm *vm; > > @@ -93,6 +93,11 @@ struct amdgpu_vm_update_params { > */ > bool override_pte; > > + /** > + * @saved_flags: Saved flags for GFP reduction. > + */ > + unsigned int saved_flags; > + > /** > * @tlb_flush_waitlist: temporary storage for BOs until tlb_flush > */ > @@ -126,21 +131,37 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device > *adev, struct amdgpu_vm_update_params *params); > int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm > *vm); > > -/* > - * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS > - * happens while holding this lock anywhere to prevent deadlocks when > - * an MMU notifier runs in reclaim-FS context. > +/** > + * amdgpu_vm_begin_critical - start the critical section of the update > + * @p: The update parameters > + * > + * Serialize all updates, check parameters and make sure that memory > allocations + * don't enter the reclaim path so that we don't deadlock with > MMU notifiers. + * > + * Returns: > + * > + * 0 on success or a negative error code on failure. > + * Even on error amdgpu_vm_end_critical() must still be called to clean up! > */ > -static inline void amdgpu_vm_eviction_lock(struct amdgpu_vm *vm) > +static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params > *p) { > - mutex_lock(&vm->eviction_lock); > - vm->saved_flags = memalloc_noreclaim_save(); > + mutex_lock(&p->vm->eviction_lock); > + p->saved_flags = memalloc_noreclaim_save(); > + if (p->vm->evicting) > + return -EBUSY; > + return 0; > } > > -static inline void amdgpu_vm_eviction_unlock(struct amdgpu_vm *vm) > +/** > + * amdgpu_vm_end_critical - end the critical section of the update > + * @p: The update parameters > + * > + * Restore the GFP flags and drop the lock. > + */ > +static inline void amdgpu_vm_end_critical(struct amdgpu_vm_update_params > *p) { > - memalloc_noreclaim_restore(vm->saved_flags); > - mutex_unlock(&vm->eviction_lock); > + memalloc_noreclaim_restore(p->saved_flags); > + mutex_unlock(&p->vm->eviction_lock); > } > > #endif > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index > 1cdf2b854f261..c03327f1242d3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > @@ -505,52 +505,51 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, > struct amdgpu_vm *vm, /** > * amdgpu_vm_pt_alloc - Allocate a specific page table > * > - * @adev: amdgpu_device pointer > - * @vm: VM to allocate page tables for > + * @p: see amdgpu_vm_update_params definition > * @cursor: Which page table to allocate > - * @immediate: use an immediate update > * > * Make sure a specific page table or directory is allocated. > * > * Returns: > - * 1 if page table needed to be allocated, 0 if page table was already > - * allocated, negative errno if an error occurred. > + * > + * 0 on success or a negative error code on failure. > */ > -static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev, > - struct amdgpu_vm *vm, > - struct amdgpu_vm_pt_cursor *cursor, > - bool immediate) > +static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p, > + struct amdgpu_vm_pt_cursor *cursor) > { > struct amdgpu_vm_bo_base *entry = cursor->entry; > struct amdgpu_bo *pt_bo; > struct amdgpu_bo_vm *pt; > - int r; > + int r, r2; > > if (entry->bo) > return 0; > > - amdgpu_vm_eviction_unlock(vm); > - r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt, > - vm->root.bo->xcp_id); > - amdgpu_vm_eviction_lock(vm); > + amdgpu_vm_end_critical(p); > + r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, p- >immediate, > + &pt, p->vm->root.bo->xcp_id); > + r2 = amdgpu_vm_begin_critical(p); > if (r) > return r; > + if (r2) > + goto error_free_pt; > > /* Keep a reference to the root directory to avoid > * freeing them up in the wrong order. > */ > pt_bo = &pt->bo; > pt_bo->parent = amdgpu_bo_ref(cursor->parent->bo); > - amdgpu_vm_bo_base_init(entry, vm, pt_bo); > - r = amdgpu_vm_pt_clear(adev, vm, pt, immediate); > + amdgpu_vm_bo_base_init(entry, p->vm, pt_bo); > + r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate); > if (r) > - goto error_free_pt; > + goto error_unpin; > > return 0; > > -error_free_pt: > - if (vm->is_npa) > +error_unpin: > + if (p->vm->is_npa) > amdgpu_bo_unpin(pt_bo); > +error_free_pt: > amdgpu_bo_unref(&pt_bo); > return r; > } > @@ -838,8 +837,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params > *params, /* make sure that the page tables covering the > * address range are actually allocated > */ > - r = amdgpu_vm_pt_alloc(params->adev, params- >vm, > - &cursor, params->immediate); > + r = amdgpu_vm_pt_alloc(params, &cursor); > if (r) > return r; > }
