[PATCH v1 1/2] nouveau/svm: fix missing folio unlock + put after make_device_exclusive_range()

[Alistair Popple]

On Fri, Jan 24, 2025 at 07:15:23PM +0100, David Hildenbrand wrote:
> In case we have to retry the loop, we are missing to unlock+put the
> folio. In that case, we will keep failing make_device_exclusive_range()
> because we cannot grab the folio lock, and even return from the function
> with the folio locked and referenced, effectively never succeeding the
> make_device_exclusive_range().
> 
> While at it, convert the other unlock+put to use a folio as well.
> 
> This was found by code inspection.

I couldn't (easily at least) trigger this condition from userspace, probably
because the window is pretty tight between mmu_interval_read_begin() and
mmu_interval_read_retry(). But I tested it by manually forcing at least one
retry and it fixes the issue observed via inspection. There's no reason to think
it would be entirely impossible to hit either, and the change looks good so
please add:

Reviewed-by: Alistair Popple <apopple at nvidia.com>
Tested-by: Alistair Popple <apopple at nvidia.com>

> Fixes: 8f187163eb89 ("nouveau/svm: implement atomic SVM access")
> Signed-off-by: David Hildenbrand <david at redhat.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_svm.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index b4da82ddbb6b2..8ea98f06d39af 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -590,6 +590,7 @@ static int nouveau_atomic_range_fault(struct nouveau_svmm *svmm,
>  	unsigned long timeout =
>  		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>  	struct mm_struct *mm = svmm->notifier.mm;
> +	struct folio *folio;
>  	struct page *page;
>  	unsigned long start = args->p.addr;
>  	unsigned long notifier_seq;
> @@ -616,12 +617,16 @@ static int nouveau_atomic_range_fault(struct nouveau_svmm *svmm,
>  			ret = -EINVAL;
>  			goto out;
>  		}
> +		folio = page_folio(page);
>  
>  		mutex_lock(&svmm->mutex);
>  		if (!mmu_interval_read_retry(&notifier->notifier,
>  					     notifier_seq))
>  			break;
>  		mutex_unlock(&svmm->mutex);
> +
> +		folio_unlock(folio);
> +		folio_put(folio);
>  	}
>  
>  	/* Map the page on the GPU. */
> @@ -637,8 +642,8 @@ static int nouveau_atomic_range_fault(struct nouveau_svmm *svmm,
>  	ret = nvif_object_ioctl(&svmm->vmm->vmm.object, args, size, NULL);
>  	mutex_unlock(&svmm->mutex);
>  
> -	unlock_page(page);
> -	put_page(page);
> +	folio_unlock(folio);
> +	folio_put(folio);
>  
>  out:
>  	mmu_interval_notifier_remove(&notifier->notifier);
> -- 
> 2.47.1
>