diff mbox

drm/radeon: Make sure radeon_vm_bo_set_addr always unreserves the BO

Message ID 1434015518-2484-1-git-send-email-michel@daenzer.net (mailing list archive)
State New, archived
Headers show

Commit Message

Michel Dänzer June 11, 2015, 9:38 a.m. UTC
From: Michel Dänzer <michel.daenzer@amd.com>

Some error paths didn't unreserve the BO. This resulted in a deadlock
down the road on the next attempt to reserve the (still reserved) BO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90873
Cc: stable@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_vm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Christian König June 11, 2015, 10:03 a.m. UTC | #1
On 11.06.2015 11:38, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Some error paths didn't unreserve the BO. This resulted in a deadlock
> down the road on the next attempt to reserve the (still reserved) BO.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90873
> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

I've already did the same change for amdgpu. Looks like I've forgotten 
to port it back to Radeon, crap!

Patch is Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_vm.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
> index de42fc4..9c3377c 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -458,14 +458,16 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>   		/* make sure object fit at this offset */
>   		eoffset = soffset + size;
>   		if (soffset >= eoffset) {
> -			return -EINVAL;
> +			r = -EINVAL;
> +			goto error_unreserve;
>   		}
>   
>   		last_pfn = eoffset / RADEON_GPU_PAGE_SIZE;
>   		if (last_pfn > rdev->vm_manager.max_pfn) {
>   			dev_err(rdev->dev, "va above limit (0x%08X > 0x%08X)\n",
>   				last_pfn, rdev->vm_manager.max_pfn);
> -			return -EINVAL;
> +			r = -EINVAL;
> +			goto error_unreserve;
>   		}
>   
>   	} else {
> @@ -486,7 +488,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>   				"(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo,
>   				soffset, tmp->bo, tmp->it.start, tmp->it.last);
>   			mutex_unlock(&vm->mutex);
> -			return -EINVAL;
> +			r = -EINVAL;
> +			goto error_unreserve;
>   		}
>   	}
>   
> @@ -497,7 +500,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>   			tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>   			if (!tmp) {
>   				mutex_unlock(&vm->mutex);
> -				return -ENOMEM;
> +				r = -ENOMEM;
> +				goto error_unreserve;
>   			}
>   			tmp->it.start = bo_va->it.start;
>   			tmp->it.last = bo_va->it.last;
> @@ -555,7 +559,6 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>   		r = radeon_vm_clear_bo(rdev, pt);
>   		if (r) {
>   			radeon_bo_unref(&pt);
> -			radeon_bo_reserve(bo_va->bo, false);
>   			return r;
>   		}
>   
> @@ -575,6 +578,10 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>   
>   	mutex_unlock(&vm->mutex);
>   	return 0;
> +
> +error_unreserve:
> +	radeon_bo_unreserve(bo_va->bo);
> +	return r;
>   }
>   
>   /**
Alex Deucher June 11, 2015, 3:11 p.m. UTC | #2
On Thu, Jun 11, 2015 at 5:38 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Some error paths didn't unreserve the BO. This resulted in a deadlock
> down the road on the next attempt to reserve the (still reserved) BO.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90873
> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Applied to my -fixes tree.

Thanks,

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_vm.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
> index de42fc4..9c3377c 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -458,14 +458,16 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>                 /* make sure object fit at this offset */
>                 eoffset = soffset + size;
>                 if (soffset >= eoffset) {
> -                       return -EINVAL;
> +                       r = -EINVAL;
> +                       goto error_unreserve;
>                 }
>
>                 last_pfn = eoffset / RADEON_GPU_PAGE_SIZE;
>                 if (last_pfn > rdev->vm_manager.max_pfn) {
>                         dev_err(rdev->dev, "va above limit (0x%08X > 0x%08X)\n",
>                                 last_pfn, rdev->vm_manager.max_pfn);
> -                       return -EINVAL;
> +                       r = -EINVAL;
> +                       goto error_unreserve;
>                 }
>
>         } else {
> @@ -486,7 +488,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>                                 "(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo,
>                                 soffset, tmp->bo, tmp->it.start, tmp->it.last);
>                         mutex_unlock(&vm->mutex);
> -                       return -EINVAL;
> +                       r = -EINVAL;
> +                       goto error_unreserve;
>                 }
>         }
>
> @@ -497,7 +500,8 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>                         tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
>                         if (!tmp) {
>                                 mutex_unlock(&vm->mutex);
> -                               return -ENOMEM;
> +                               r = -ENOMEM;
> +                               goto error_unreserve;
>                         }
>                         tmp->it.start = bo_va->it.start;
>                         tmp->it.last = bo_va->it.last;
> @@ -555,7 +559,6 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>                 r = radeon_vm_clear_bo(rdev, pt);
>                 if (r) {
>                         radeon_bo_unref(&pt);
> -                       radeon_bo_reserve(bo_va->bo, false);
>                         return r;
>                 }
>
> @@ -575,6 +578,10 @@ int radeon_vm_bo_set_addr(struct radeon_device *rdev,
>
>         mutex_unlock(&vm->mutex);
>         return 0;
> +
> +error_unreserve:
> +       radeon_bo_unreserve(bo_va->bo);
> +       return r;
>  }
>
>  /**
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
index de42fc4..9c3377c 100644
--- a/drivers/gpu/drm/radeon/radeon_vm.c
+++ b/drivers/gpu/drm/radeon/radeon_vm.c
@@ -458,14 +458,16 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 		/* make sure object fit at this offset */
 		eoffset = soffset + size;
 		if (soffset >= eoffset) {
-			return -EINVAL;
+			r = -EINVAL;
+			goto error_unreserve;
 		}
 
 		last_pfn = eoffset / RADEON_GPU_PAGE_SIZE;
 		if (last_pfn > rdev->vm_manager.max_pfn) {
 			dev_err(rdev->dev, "va above limit (0x%08X > 0x%08X)\n",
 				last_pfn, rdev->vm_manager.max_pfn);
-			return -EINVAL;
+			r = -EINVAL;
+			goto error_unreserve;
 		}
 
 	} else {
@@ -486,7 +488,8 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 				"(bo %p 0x%010lx 0x%010lx)\n", bo_va->bo,
 				soffset, tmp->bo, tmp->it.start, tmp->it.last);
 			mutex_unlock(&vm->mutex);
-			return -EINVAL;
+			r = -EINVAL;
+			goto error_unreserve;
 		}
 	}
 
@@ -497,7 +500,8 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 			tmp = kzalloc(sizeof(struct radeon_bo_va), GFP_KERNEL);
 			if (!tmp) {
 				mutex_unlock(&vm->mutex);
-				return -ENOMEM;
+				r = -ENOMEM;
+				goto error_unreserve;
 			}
 			tmp->it.start = bo_va->it.start;
 			tmp->it.last = bo_va->it.last;
@@ -555,7 +559,6 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 		r = radeon_vm_clear_bo(rdev, pt);
 		if (r) {
 			radeon_bo_unref(&pt);
-			radeon_bo_reserve(bo_va->bo, false);
 			return r;
 		}
 
@@ -575,6 +578,10 @@  int radeon_vm_bo_set_addr(struct radeon_device *rdev,
 
 	mutex_unlock(&vm->mutex);
 	return 0;
+
+error_unreserve:
+	radeon_bo_unreserve(bo_va->bo);
+	return r;
 }
 
 /**