Re: [PATCH v3 3/3] drm/amdgpu: simplify reservation handling during buffer creation

2017-02-16 Thread zhoucm1 



On 2017年02月16日 20:08, Christian König wrote:

Am 16.02.2017 um 12:39 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

By using ttm_bo_init_reserved instead of the manual initialization of
the reservation object, the reservation lock will be properly unlocked
and destroyed when the TTM BO initialization fails.

Actual deadlocks caused by the missing unlock should have been fixed
by "drm/ttm: never add BO that failed to validate to the LRU list",
superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
a potential deadlock in amdgpu_bo_create_restricted()").

This change fixes remaining recursive locking errors that can be seen
with lock debugging enabled, and avoids the error of freeing a locked
mutex.

As an additional minor bonus, buffers created with resv == NULL and
the AMDGPU_GEM_CREATE_VRAM_CLEARED flag are now only added to the
global LRU list after the fill commands have been issued.

Fixes: 12a852219583 ("drm/amdgpu: improve 
AMDGPU_GEM_CREATE_VRAM_CLEARED handling (v2)")

Signed-off-by: Nicolai Hähnle 

Reviewed-by: Chunming Zhou  as well


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 17 -
  1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index c2e57f7..4d0536d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -395,19 +395,10 @@ int amdgpu_bo_create_restricted(struct 
amdgpu_device *adev,

  amdgpu_fill_placement_to_bo(bo, placement);
  /* Kernel allocation are uninterruptible */
  -if (!resv) {
-bool locked;
-
-reservation_object_init(>tbo.ttm_resv);
-locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
-WARN_ON(!locked);
-}
-
  initial_bytes_moved = atomic64_read(>num_bytes_moved);
-r = ttm_bo_init(>mman.bdev, >tbo, size, type,
->placement, page_align, !kernel, NULL,
-acc_size, sg, resv ? resv : >tbo.ttm_resv,
-_ttm_bo_destroy);
+r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+ >placement, page_align, !kernel, NULL,
+ acc_size, sg, resv, _ttm_bo_destroy);
  amdgpu_cs_report_moved_bytes(adev,
  atomic64_read(>num_bytes_moved) - initial_bytes_moved);
  @@ -433,7 +424,7 @@ int amdgpu_bo_create_restricted(struct 
amdgpu_device *adev,

  fence_put(fence);
  }
  if (!resv)
-ww_mutex_unlock(>tbo.resv->lock);
+ttm_bo_unreserve(>tbo);


You can just use amdgpu_bo_unreserve() here. With that fixed the whole 
set is Reviewed-by: Christian König .


Regards,
Christian.


  *bo_ptr = bo;
trace_amdgpu_bo_create(bo);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/3] drm/amdgpu: simplify reservation handling during buffer creation

2017-02-16 Thread Christian König 

Am 16.02.2017 um 12:39 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

By using ttm_bo_init_reserved instead of the manual initialization of
the reservation object, the reservation lock will be properly unlocked
and destroyed when the TTM BO initialization fails.

Actual deadlocks caused by the missing unlock should have been fixed
by "drm/ttm: never add BO that failed to validate to the LRU list",
superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
a potential deadlock in amdgpu_bo_create_restricted()").

This change fixes remaining recursive locking errors that can be seen
with lock debugging enabled, and avoids the error of freeing a locked
mutex.

As an additional minor bonus, buffers created with resv == NULL and
the AMDGPU_GEM_CREATE_VRAM_CLEARED flag are now only added to the
global LRU list after the fill commands have been issued.

Fixes: 12a852219583 ("drm/amdgpu: improve AMDGPU_GEM_CREATE_VRAM_CLEARED handling 
(v2)")
Signed-off-by: Nicolai Hähnle 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 17 -
  1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index c2e57f7..4d0536d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -395,19 +395,10 @@ int amdgpu_bo_create_restricted(struct amdgpu_device 
*adev,
amdgpu_fill_placement_to_bo(bo, placement);
/* Kernel allocation are uninterruptible */
  
-	if (!resv) {

-   bool locked;
-
-   reservation_object_init(>tbo.ttm_resv);
-   locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
-   WARN_ON(!locked);
-   }
-
initial_bytes_moved = atomic64_read(>num_bytes_moved);
-   r = ttm_bo_init(>mman.bdev, >tbo, size, type,
-   >placement, page_align, !kernel, NULL,
-   acc_size, sg, resv ? resv : >tbo.ttm_resv,
-   _ttm_bo_destroy);
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+>placement, page_align, !kernel, NULL,
+acc_size, sg, resv, _ttm_bo_destroy);
amdgpu_cs_report_moved_bytes(adev,
atomic64_read(>num_bytes_moved) - initial_bytes_moved);
  
@@ -433,7 +424,7 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,

fence_put(fence);
}
if (!resv)
-   ww_mutex_unlock(>tbo.resv->lock);
+   ttm_bo_unreserve(>tbo);


You can just use amdgpu_bo_unreserve() here. With that fixed the whole 
set is Reviewed-by: Christian König .


Regards,
Christian.


*bo_ptr = bo;
  
  	trace_amdgpu_bo_create(bo);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/3] drm/amdgpu: simplify reservation handling during buffer creation

2017-02-16 Thread Nicolai Hähnle 
From: Nicolai Hähnle 

By using ttm_bo_init_reserved instead of the manual initialization of
the reservation object, the reservation lock will be properly unlocked
and destroyed when the TTM BO initialization fails.

Actual deadlocks caused by the missing unlock should have been fixed
by "drm/ttm: never add BO that failed to validate to the LRU list",
superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
a potential deadlock in amdgpu_bo_create_restricted()").

This change fixes remaining recursive locking errors that can be seen
with lock debugging enabled, and avoids the error of freeing a locked
mutex.

As an additional minor bonus, buffers created with resv == NULL and
the AMDGPU_GEM_CREATE_VRAM_CLEARED flag are now only added to the
global LRU list after the fill commands have been issued.

Fixes: 12a852219583 ("drm/amdgpu: improve AMDGPU_GEM_CREATE_VRAM_CLEARED 
handling (v2)")
Signed-off-by: Nicolai Hähnle 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index c2e57f7..4d0536d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -395,19 +395,10 @@ int amdgpu_bo_create_restricted(struct amdgpu_device 
*adev,
amdgpu_fill_placement_to_bo(bo, placement);
/* Kernel allocation are uninterruptible */
 
-   if (!resv) {
-   bool locked;
-
-   reservation_object_init(>tbo.ttm_resv);
-   locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
-   WARN_ON(!locked);
-   }
-
initial_bytes_moved = atomic64_read(>num_bytes_moved);
-   r = ttm_bo_init(>mman.bdev, >tbo, size, type,
-   >placement, page_align, !kernel, NULL,
-   acc_size, sg, resv ? resv : >tbo.ttm_resv,
-   _ttm_bo_destroy);
+   r = ttm_bo_init_reserved(>mman.bdev, >tbo, size, type,
+>placement, page_align, !kernel, NULL,
+acc_size, sg, resv, _ttm_bo_destroy);
amdgpu_cs_report_moved_bytes(adev,
atomic64_read(>num_bytes_moved) - initial_bytes_moved);
 
@@ -433,7 +424,7 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
fence_put(fence);
}
if (!resv)
-   ww_mutex_unlock(>tbo.resv->lock);
+   ttm_bo_unreserve(>tbo);
*bo_ptr = bo;
 
trace_amdgpu_bo_create(bo);
-- 
2.9.3

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel