Re: [PATCH V2] accel/amdxdna: Fix unexpected wait when flushing notifier_wq

[Max Zhen <max.zhen@amd.com>]

On 8/13/2026 Thu 13:50, Lizhi Hou wrote:
> In amdxdna_gem_obj_free(), flush_workqueue(xdna->notifier_wq) waits for
> all pending work items on the device-global notifier workqueue, rather
> than only the work items associated with the BO being freed.
> 
> If another BO has a pending hmm_unreg_work, freeing an unrelated BO can
> be unnecessarily blocked until that work completes.
> 
> mmu_interval_notifier_remove() is deferred to a workqueue because it
> cannot be called from the MMU notifier callback itself. The BO free path
> is not a notifier callback, so call mmu_interval_notifier_remove()
> directly there and avoid the workqueue.
> 
> Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
> V2:
>    Fix sashiko comment.
> 
>   drivers/accel/amdxdna/amdxdna_gem.c | 69 +++++++++++++++++++----------
>   drivers/accel/amdxdna/amdxdna_gem.h |  1 +
>   2 files changed, 47 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 1c63eff0a4a8..1c17122d1327 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -253,13 +253,16 @@ static bool amdxdna_hmm_invalidate(struct mmu_interval_notifier *mni,
>   	struct amdxdna_gem_obj *abo = mapp->abo;
>   	struct amdxdna_dev *xdna;
>   
> +	if (!mmu_notifier_range_blockable(range))
> +		return false;
> +
> +	if (mapp->unmapped)
> +		return true;
> +
>   	xdna = to_xdna_dev(to_gobj(abo)->dev);
>   	XDNA_DBG(xdna, "Invalidating range 0x%lx, 0x%lx, type %d",
>   		 mapp->range.start, mapp->range.end, abo->type);
>   
> -	if (!mmu_notifier_range_blockable(range))
> -		return false;
> -
>   	down_write(&xdna->notifier_lock);
>   	abo->mem.map_invalid = true;
>   	mapp->invalid = true;
> @@ -301,33 +304,40 @@ static void amdxdna_hmm_unregister(struct amdxdna_gem_obj *abo,
>   
>   	down_write(&xdna->notifier_lock);
>   	list_for_each_entry(mapp, &abo->mem.umap_list, node) {
> -		if (!vma || compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end)) {
> -			if (!mapp->unmapped) {
> -				queue_work(xdna->notifier_wq, &mapp->hmm_unreg_work);
> -				mapp->unmapped = true;
> -			}
> -			if (vma)
> -				break;
> -		}
> +		if (!compare_range(mapp, vma->vm_mm, vma->vm_start, vma->vm_end))
> +			continue;
> +
> +		queue_work(xdna->notifier_wq, &mapp->hmm_unreg_work);
> +		mapp->unmapped = true;
>   	}
>   	up_write(&xdna->notifier_lock);
>   }
>   
> -static void amdxdna_umap_release(struct kref *ref)
> +static void amdxdna_hmm_unregister_all(struct amdxdna_gem_obj *abo)
>   {
> -	struct amdxdna_umap *mapp = container_of(ref, struct amdxdna_umap, refcnt);
> -	struct amdxdna_gem_obj *abo = mapp->abo;
> -	struct amdxdna_dev *xdna;
> -
> -	mmu_interval_notifier_remove(&mapp->notifier);
> +	struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
> +	struct amdxdna_umap *mapp, *tmp;
> +	LIST_HEAD(dead);
>   
> -	xdna = to_xdna_dev(to_gobj(mapp->abo)->dev);
>   	down_write(&xdna->notifier_lock);
> -	list_del(&mapp->node);
> -	if (list_empty(&abo->mem.umap_list))
> -		abo->mem.uva = AMDXDNA_INVALID_ADDR;
> +	list_for_each_entry_safe(mapp, tmp, &abo->mem.umap_list, node) {
> +		mapp->unmapped = true;
> +		mapp->cleanup = true;
> +		list_move(&mapp->node, &dead);
> +	}
>   	up_write(&xdna->notifier_lock);
>   
> +	list_for_each_entry_safe(mapp, tmp, &dead, node) {
> +		cancel_work_sync(&mapp->hmm_unreg_work);
> +		amdxdna_umap_put(mapp);
> +	}
> +}
> +
> +static void amdxdna_umap_release(struct kref *ref)
> +{
> +	struct amdxdna_umap *mapp = container_of(ref, struct amdxdna_umap, refcnt);
> +
> +	mmu_interval_notifier_remove(&mapp->notifier);
>   	kvfree(mapp->range.hmm_pfns);
>   	kfree(mapp);
>   }
> @@ -341,6 +351,20 @@ static void amdxdna_hmm_unreg_work(struct work_struct *work)
>   {
>   	struct amdxdna_umap *mapp = container_of(work, struct amdxdna_umap,
>   						 hmm_unreg_work);
> +	struct amdxdna_gem_obj *abo = mapp->abo;
> +	struct amdxdna_dev *xdna;
> +
> +	xdna = to_xdna_dev(to_gobj(mapp->abo)->dev);
> +	down_write(&xdna->notifier_lock);
> +	if (mapp->cleanup) {
> +		up_write(&xdna->notifier_lock);
> +		return;
> +	}
> +
> +	list_del(&mapp->node);
> +	if (list_empty(&abo->mem.umap_list))
> +		abo->mem.uva = AMDXDNA_INVALID_ADDR;
> +	up_write(&xdna->notifier_lock);
>   
>   	amdxdna_umap_put(mapp);
>   }
> @@ -643,8 +667,7 @@ static void amdxdna_gem_obj_free(struct drm_gem_object *gobj)
>   	struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
>   
> -	amdxdna_hmm_unregister(abo, NULL);
> -	flush_workqueue(xdna->notifier_wq);
> +	amdxdna_hmm_unregister_all(abo);
>   
>   	if (abo->pinned)
>   		amdxdna_gem_unpin(abo);
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.h b/drivers/accel/amdxdna/amdxdna_gem.h
> index 1e90e32bf3cd..fb033ced1045 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.h
> +++ b/drivers/accel/amdxdna/amdxdna_gem.h
> @@ -20,6 +20,7 @@ struct amdxdna_umap {
>   	struct kref			refcnt;
>   	bool				invalid;
>   	bool				unmapped;
> +	bool				cleanup;
>   };
>   
>   struct amdxdna_mem {