diff mbox series

[v2,2/4] drm/xe/vm: Implement userptr page pinning

Message ID 20230822162136.25895-3-thomas.hellstrom@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/xe: Support optional pinning of userptr pages | expand

Commit Message

Thomas Hellstrom Aug. 22, 2023, 4:21 p.m. UTC 
Implement pinning of userptrs between VM_BIND and VM_UNBIND, which will
facilitate avoiding long hangs on non-preemptible workloads. But don't
hook it up to userspace just yet.

v2:
- Avoid marking userptr VMAs as invalid in the mmu invalidation notifier.
  (Matthew Brost)
- Add an WARN that we don't try to repin userptr pages (Matthew Brost)

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_vm.c       | 80 +++++++++++++++++++++++---------
 drivers/gpu/drm/xe/xe_vm.h       |  9 ++++
 drivers/gpu/drm/xe/xe_vm_types.h | 12 +++++
 3 files changed, 79 insertions(+), 22 deletions(-)

Comments

Matthew Brost Aug. 22, 2023, 11:58 p.m. UTC | #1 
On Tue, Aug 22, 2023 at 06:21:34PM +0200, Thomas Hellström wrote:
> Implement pinning of userptrs between VM_BIND and VM_UNBIND, which will
> facilitate avoiding long hangs on non-preemptible workloads. But don't
> hook it up to userspace just yet.
> 
> v2:
> - Avoid marking userptr VMAs as invalid in the mmu invalidation notifier.
>   (Matthew Brost)
> - Add an WARN that we don't try to repin userptr pages (Matthew Brost)
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_vm.c       | 80 +++++++++++++++++++++++---------
>  drivers/gpu/drm/xe/xe_vm.h       |  9 ++++
>  drivers/gpu/drm/xe/xe_vm_types.h | 12 +++++
>  3 files changed, 79 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 8bf7f62e6548..037ac42f74a5 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -74,10 +74,6 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
>  	if (notifier_seq == vma->userptr.notifier_seq)
>  		return 0;
>  
> -	pages = kvmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
> -	if (!pages)
> -		return -ENOMEM;
> -
>  	if (vma->userptr.sg) {
>  		dma_unmap_sgtable(xe->drm.dev,
>  				  vma->userptr.sg,
> @@ -87,6 +83,18 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
>  		vma->userptr.sg = NULL;
>  	}
>  
> +	/* TODO: Convert to xe_assert() */
> +	if (XE_WARN_ON(vma->userptr.pinned_pages)) {
> +		unpin_user_pages_dirty_lock(vma->userptr.pinned_pages,
> +					    vma->userptr.num_pinned,
> +					    !read_only);
> +		pages = vma->userptr.pinned_pages;
> +	} else {
> +		pages = kvmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
> +		if (!pages)
> +			return -ENOMEM;
> +	}
> +
>  	pinned = ret = 0;
>  	if (in_kthread) {
>  		if (!mmget_not_zero(vma->userptr.notifier.mm)) {
> @@ -97,11 +105,18 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
>  	}
>  
>  	while (pinned < num_pages) {
> -		ret = get_user_pages_fast(xe_vma_userptr(vma) +
> -					  pinned * PAGE_SIZE,
> -					  num_pages - pinned,
> -					  read_only ? 0 : FOLL_WRITE,
> -					  &pages[pinned]);
> +		if (xe_vma_is_pinned(vma))
> +			ret = pin_user_pages_fast(xe_vma_userptr(vma) +
> +						  pinned * PAGE_SIZE,
> +						  num_pages - pinned,
> +						  read_only ? 0 : FOLL_WRITE,
> +						  &pages[pinned]);
> +		else
> +			ret = get_user_pages_fast(xe_vma_userptr(vma) +
> +						  pinned * PAGE_SIZE,
> +						  num_pages - pinned,
> +						  read_only ? 0 : FOLL_WRITE,
> +						  &pages[pinned]);
>  		if (ret < 0) {
>  			if (in_kthread)
>  				ret = 0;
> @@ -137,19 +152,24 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
>  	if (ret)
>  		goto out_free_sg;
>  
> -	for (i = 0; i < pinned; ++i) {
> -		if (!read_only) {
> -			lock_page(pages[i]);
> -			set_page_dirty(pages[i]);
> -			unlock_page(pages[i]);
> +	if (!xe_vma_is_pinned(vma)) {
> +		for (i = 0; i < pinned; ++i) {
> +			if (!read_only) {
> +				lock_page(pages[i]);
> +				set_page_dirty(pages[i]);
> +				unlock_page(pages[i]);
> +			}
> +
> +			mark_page_accessed(pages[i]);
>  		}
>  
> -		mark_page_accessed(pages[i]);
> +		release_pages(pages, pinned);
> +		kvfree(pages);
> +	} else {
> +		vma->userptr.pinned_pages = pages;
> +		vma->userptr.num_pinned = pinned;
>  	}
>  
> -	release_pages(pages, pinned);
> -	kvfree(pages);
> -
>  	vma->userptr.notifier_seq = notifier_seq;
>  	if (xe_vma_userptr_check_repin(vma) == -EAGAIN)
>  		goto retry;
> @@ -160,9 +180,14 @@ int xe_vma_userptr_pin_pages(struct xe_vma *vma)
>  	sg_free_table(vma->userptr.sg);
>  	vma->userptr.sg = NULL;
>  out_release_pages:
> -	release_pages(pages, pinned);
> +	if (!xe_vma_is_pinned(vma))
> +		release_pages(pages, pinned);
> +	else
> +		unpin_user_pages(pages, pinned);
> +	vma->userptr.num_pinned = 0;
>  mm_closed:
>  	kvfree(pages);
> +	vma->userptr.pinned_pages = NULL;
>  	return ret;
>  }
>  
> @@ -718,6 +743,11 @@ static bool vma_userptr_invalidate(struct mmu_interval_notifier *mni,
>  		return false;
>  
>  	down_write(&vm->userptr.notifier_lock);
> +	if (xe_vma_is_pinned(vma)) {
> +		up_write(&vm->userptr.notifier_lock);
> +		return true;
> +	}
> +
>  	mmu_interval_set_seq(mni, cur_seq);
>  
>  	/* No need to stop gpu access if the userptr is not yet bound. */
> @@ -976,10 +1006,16 @@ static void xe_vma_destroy_late(struct xe_vma *vma)
>  			vma->userptr.sg = NULL;
>  		}
>  
> +		if (vma->userptr.pinned_pages) {
> +			unpin_user_pages_dirty_lock(vma->userptr.pinned_pages,
> +						    vma->userptr.num_pinned,
> +						    !read_only);
> +			kvfree(vma->userptr.pinned_pages);
> +		}
> +
>  		/*
> -		 * Since userptr pages are not pinned, we can't remove
> -		 * the notifer until we're sure the GPU is not accessing
> -		 * them anymore
> +		 * We can't remove the notifer until we're sure the GPU is
> +		 * not accessing the pages anymore
>  		 */
>  		mmu_interval_notifier_remove(&vma->userptr.notifier);
>  		xe_vm_put(vm);
> diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
> index 6de6e3edb24a..913544d7d995 100644
> --- a/drivers/gpu/drm/xe/xe_vm.h
> +++ b/drivers/gpu/drm/xe/xe_vm.h
> @@ -139,6 +139,15 @@ static inline bool xe_vma_is_userptr(struct xe_vma *vma)
>  	return xe_vma_has_no_bo(vma) && !xe_vma_is_null(vma);
>  }
>  
> +/**
> + * xe_vma_is_pinned() - User has requested the backing store of this vma
> + * to be pinned.
> + */
> +static inline bool xe_vma_is_pinned(struct xe_vma *vma)
> +{
> +	return xe_vma_is_userptr(vma) && (vma->gpuva.flags & XE_VMA_PINNED);
> +}
> +
>  #define xe_vm_assert_held(vm) dma_resv_assert_held(&(vm)->resv)
>  
>  u64 xe_vm_pdp4_descriptor(struct xe_vm *vm, struct xe_tile *tile);
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> index 3681a5ff588b..9b90e649cd69 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -33,6 +33,8 @@ struct xe_vm;
>  #define XE_VMA_PTE_4K		(DRM_GPUVA_USERBITS << 5)
>  #define XE_VMA_PTE_2M		(DRM_GPUVA_USERBITS << 6)
>  #define XE_VMA_PTE_1G		(DRM_GPUVA_USERBITS << 7)
> +/* User requested backing store to be pinned */
> +#define XE_VMA_PINNED           (DRM_GPUVA_USERBITS << 8)
>  
>  /** struct xe_userptr - User pointer */
>  struct xe_userptr {
> @@ -54,6 +56,16 @@ struct xe_userptr {
>  	 * read: vm->userptr.notifier_lock in write mode or vm->resv held.
>  	 */
>  	bool initial_bind;
> +	/**
> +	 * @pinned_pages: List of pinned pages if xe_vma_pinned(),
> +	 * NULL otherwise. protected by the vm lock.
> +	 */
> +	struct page **pinned_pages;
> +	/**
> +	 * @num_pinned: Number of pointers to pinned pages in @pinned_pages.
> +	 * protected by the vm lock.
> +	 */
> +	unsigned long num_pinned;
>  #if IS_ENABLED(CONFIG_DRM_XE_USERPTR_INVAL_INJECT)
>  	u32 divisor;
>  #endif
> -- 
> 2.41.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 8bf7f62e6548..037ac42f74a5 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -74,10 +74,6 @@  int xe_vma_userptr_pin_pages(struct xe_vma *vma)
 	if (notifier_seq == vma->userptr.notifier_seq)
 		return 0;
 
-	pages = kvmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
-	if (!pages)
-		return -ENOMEM;
-
 	if (vma->userptr.sg) {
 		dma_unmap_sgtable(xe->drm.dev,
 				  vma->userptr.sg,
@@ -87,6 +83,18 @@  int xe_vma_userptr_pin_pages(struct xe_vma *vma)
 		vma->userptr.sg = NULL;
 	}
 
+	/* TODO: Convert to xe_assert() */
+	if (XE_WARN_ON(vma->userptr.pinned_pages)) {
+		unpin_user_pages_dirty_lock(vma->userptr.pinned_pages,
+					    vma->userptr.num_pinned,
+					    !read_only);
+		pages = vma->userptr.pinned_pages;
+	} else {
+		pages = kvmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
+		if (!pages)
+			return -ENOMEM;
+	}
+
 	pinned = ret = 0;
 	if (in_kthread) {
 		if (!mmget_not_zero(vma->userptr.notifier.mm)) {
@@ -97,11 +105,18 @@  int xe_vma_userptr_pin_pages(struct xe_vma *vma)
 	}
 
 	while (pinned < num_pages) {
-		ret = get_user_pages_fast(xe_vma_userptr(vma) +
-					  pinned * PAGE_SIZE,
-					  num_pages - pinned,
-					  read_only ? 0 : FOLL_WRITE,
-					  &pages[pinned]);
+		if (xe_vma_is_pinned(vma))
+			ret = pin_user_pages_fast(xe_vma_userptr(vma) +
+						  pinned * PAGE_SIZE,
+						  num_pages - pinned,
+						  read_only ? 0 : FOLL_WRITE,
+						  &pages[pinned]);
+		else
+			ret = get_user_pages_fast(xe_vma_userptr(vma) +
+						  pinned * PAGE_SIZE,
+						  num_pages - pinned,
+						  read_only ? 0 : FOLL_WRITE,
+						  &pages[pinned]);
 		if (ret < 0) {
 			if (in_kthread)
 				ret = 0;
@@ -137,19 +152,24 @@  int xe_vma_userptr_pin_pages(struct xe_vma *vma)
 	if (ret)
 		goto out_free_sg;
 
-	for (i = 0; i < pinned; ++i) {
-		if (!read_only) {
-			lock_page(pages[i]);
-			set_page_dirty(pages[i]);
-			unlock_page(pages[i]);
+	if (!xe_vma_is_pinned(vma)) {
+		for (i = 0; i < pinned; ++i) {
+			if (!read_only) {
+				lock_page(pages[i]);
+				set_page_dirty(pages[i]);
+				unlock_page(pages[i]);
+			}
+
+			mark_page_accessed(pages[i]);
 		}
 
-		mark_page_accessed(pages[i]);
+		release_pages(pages, pinned);
+		kvfree(pages);
+	} else {
+		vma->userptr.pinned_pages = pages;
+		vma->userptr.num_pinned = pinned;
 	}
 
-	release_pages(pages, pinned);
-	kvfree(pages);
-
 	vma->userptr.notifier_seq = notifier_seq;
 	if (xe_vma_userptr_check_repin(vma) == -EAGAIN)
 		goto retry;
@@ -160,9 +180,14 @@  int xe_vma_userptr_pin_pages(struct xe_vma *vma)
 	sg_free_table(vma->userptr.sg);
 	vma->userptr.sg = NULL;
 out_release_pages:
-	release_pages(pages, pinned);
+	if (!xe_vma_is_pinned(vma))
+		release_pages(pages, pinned);
+	else
+		unpin_user_pages(pages, pinned);
+	vma->userptr.num_pinned = 0;
 mm_closed:
 	kvfree(pages);
+	vma->userptr.pinned_pages = NULL;
 	return ret;
 }
 
@@ -718,6 +743,11 @@  static bool vma_userptr_invalidate(struct mmu_interval_notifier *mni,
 		return false;
 
 	down_write(&vm->userptr.notifier_lock);
+	if (xe_vma_is_pinned(vma)) {
+		up_write(&vm->userptr.notifier_lock);
+		return true;
+	}
+
 	mmu_interval_set_seq(mni, cur_seq);
 
 	/* No need to stop gpu access if the userptr is not yet bound. */
@@ -976,10 +1006,16 @@  static void xe_vma_destroy_late(struct xe_vma *vma)
 			vma->userptr.sg = NULL;
 		}
 
+		if (vma->userptr.pinned_pages) {
+			unpin_user_pages_dirty_lock(vma->userptr.pinned_pages,
+						    vma->userptr.num_pinned,
+						    !read_only);
+			kvfree(vma->userptr.pinned_pages);
+		}
+
 		/*
-		 * Since userptr pages are not pinned, we can't remove
-		 * the notifer until we're sure the GPU is not accessing
-		 * them anymore
+		 * We can't remove the notifer until we're sure the GPU is
+		 * not accessing the pages anymore
 		 */
 		mmu_interval_notifier_remove(&vma->userptr.notifier);
 		xe_vm_put(vm);
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index 6de6e3edb24a..913544d7d995 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -139,6 +139,15 @@  static inline bool xe_vma_is_userptr(struct xe_vma *vma)
 	return xe_vma_has_no_bo(vma) && !xe_vma_is_null(vma);
 }
 
+/**
+ * xe_vma_is_pinned() - User has requested the backing store of this vma
+ * to be pinned.
+ */
+static inline bool xe_vma_is_pinned(struct xe_vma *vma)
+{
+	return xe_vma_is_userptr(vma) && (vma->gpuva.flags & XE_VMA_PINNED);
+}
+
 #define xe_vm_assert_held(vm) dma_resv_assert_held(&(vm)->resv)
 
 u64 xe_vm_pdp4_descriptor(struct xe_vm *vm, struct xe_tile *tile);
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 3681a5ff588b..9b90e649cd69 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -33,6 +33,8 @@  struct xe_vm;
 #define XE_VMA_PTE_4K		(DRM_GPUVA_USERBITS << 5)
 #define XE_VMA_PTE_2M		(DRM_GPUVA_USERBITS << 6)
 #define XE_VMA_PTE_1G		(DRM_GPUVA_USERBITS << 7)
+/* User requested backing store to be pinned */
+#define XE_VMA_PINNED           (DRM_GPUVA_USERBITS << 8)
 
 /** struct xe_userptr - User pointer */
 struct xe_userptr {
@@ -54,6 +56,16 @@  struct xe_userptr {
 	 * read: vm->userptr.notifier_lock in write mode or vm->resv held.
 	 */
 	bool initial_bind;
+	/**
+	 * @pinned_pages: List of pinned pages if xe_vma_pinned(),
+	 * NULL otherwise. protected by the vm lock.
+	 */
+	struct page **pinned_pages;
+	/**
+	 * @num_pinned: Number of pointers to pinned pages in @pinned_pages.
+	 * protected by the vm lock.
+	 */
+	unsigned long num_pinned;
 #if IS_ENABLED(CONFIG_DRM_XE_USERPTR_INVAL_INJECT)
 	u32 divisor;
 #endif