[PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

[PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

[Felix Kuehling]

A BO that's already swapped would be added back to the swap-LRU list
for example if its validation failed under high memory pressure. This
could later lead to swapping it out again and leaking previous swap
storage.

This commit adds a condition to prevent that from happening.

v2: Check page_flags instead of swap_storage

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2eb71ff..62518b6 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -170,7 +170,8 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
 		list_add_tail(&bo->lru, &man->lru[bo->priority]);
 		kref_get(&bo->list_kref);
 
-		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
+		if (bo->ttm && !(bo->ttm->page_flags &
+				 (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) {
 			list_add_tail(&bo->swap,
 				      &bo->glob->swap_lru[bo->priority]);
 			kref_get(&bo->list_kref);
-- 
2.7.4

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

[PATCH 2/2] drm/ttm: Don't unreserve swapped BOs that were previously reserved

[Felix Kuehling]

If ttm_bo_swapout doesn't own the lock, don't release it. Someone
else probably depends on it still being locked.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 62518b6..893003f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1780,8 +1780,8 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
 	 * Unreserve without putting on LRU to avoid swapping out an
 	 * already swapped buffer.
 	 */
-
-	reservation_object_unlock(bo->resv);
+	if (locked)
+		reservation_object_unlock(bo->resv);
 	kref_put(&bo->list_kref, ttm_bo_release_list);
 	return ret;
 }
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

[Christian König]

Am 18.01.2018 um 17:56 schrieb Felix Kuehling:
> A BO that's already swapped would be added back to the swap-LRU list
> for example if its validation failed under high memory pressure. This
> could later lead to swapping it out again and leaking previous swap
> storage.
>
> This commit adds a condition to prevent that from happening.
>
> v2: Check page_flags instead of swap_storage
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

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

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2eb71ff..62518b6 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -170,7 +170,8 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
>   		list_add_tail(&bo->lru, &man->lru[bo->priority]);
>   		kref_get(&bo->list_kref);
>   
> -		if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
> +		if (bo->ttm && !(bo->ttm->page_flags &
> +				 (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) {
>   			list_add_tail(&bo->swap,
>   				      &bo->glob->swap_lru[bo->priority]);
>   			kref_get(&bo->list_kref);

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

Re: [PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

[Felix Kuehling]

On 2018-01-18 01:09 PM, Christian König wrote:
> Am 18.01.2018 um 17:56 schrieb Felix Kuehling:
>> A BO that's already swapped would be added back to the swap-LRU list
>> for example if its validation failed under high memory pressure. This
>> could later lead to swapping it out again and leaking previous swap
>> storage.
>>
>> This commit adds a condition to prevent that from happening.
>>
>> v2: Check page_flags instead of swap_storage
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com> for both.

Thanks. I pushed them to amd-staging-drm-next. I think patch 1 should
also go to -fixes. Alex?

Regards,
  Felix

>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 2eb71ff..62518b6 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -170,7 +170,8 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
>>           list_add_tail(&bo->lru, &man->lru[bo->priority]);
>>           kref_get(&bo->list_kref);
>>   -        if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> +        if (bo->ttm && !(bo->ttm->page_flags &
>> +                 (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) {
>>               list_add_tail(&bo->swap,
>>                         &bo->glob->swap_lru[bo->priority]);
>>               kref_get(&bo->list_kref);
>

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

Re: [PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 1975 bytes --]


________________________________
From: Kuehling, Felix
Sent: Thursday, January 18, 2018 4:12 PM
To: Koenig, Christian; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; dri-devel-PD4FTy7X32mMSPqsTGOZug@public.gmane.orgesktop.org; He, Roger; Deucher, Alexander
Subject: Re: [PATCH 1/2] drm/ttm: Don't add swapped BOs to swap-LRU list

On 2018-01-18 01:09 PM, Christian König wrote:
> Am 18.01.2018 um 17:56 schrieb Felix Kuehling:
>> A BO that's already swapped would be added back to the swap-LRU list
>> for example if its validation failed under high memory pressure. This
>> could later lead to swapping it out again and leaking previous swap
>> storage.
>>
>> This commit adds a condition to prevent that from happening.
>>
>> v2: Check page_flags instead of swap_storage
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>
>
> Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org> for both.

Thanks. I pushed them to amd-staging-drm-next. I think patch 1 should
also go to -fixes. Alex?

Yup, already planning on it.

Alex

Regards,
  Felix

>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 2eb71ff..62518b6 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -170,7 +170,8 @@ void ttm_bo_add_to_lru(struct ttm_buffer_object *bo)
>>           list_add_tail(&bo->lru, &man->lru[bo->priority]);
>>           kref_get(&bo->list_kref);
>>   -        if (bo->ttm && !(bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>> +        if (bo->ttm && !(bo->ttm->page_flags &
>> +                 (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED))) {
>>               list_add_tail(&bo->swap,
>>                         &bo->glob->swap_lru[bo->priority]);
>>               kref_get(&bo->list_kref);
>


[-- Attachment #1.2: Type: text/html, Size: 4064 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/ttm: Don't unreserve swapped BOs that were previously reserved

[He, Roger]

good catch! 

Thanks
Roger(Hongbo.He)
-----Original Message-----
From: Kuehling, Felix 
Sent: Friday, January 19, 2018 12:56 AM
To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; He, Roger <Hongbo.He@amd.com>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>
Subject: [PATCH 2/2] drm/ttm: Don't unreserve swapped BOs that were previously reserved

If ttm_bo_swapout doesn't own the lock, don't release it. Someone else probably depends on it still being locked.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 62518b6..893003f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1780,8 +1780,8 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
 	 * Unreserve without putting on LRU to avoid swapping out an
 	 * already swapped buffer.
 	 */
-
-	reservation_object_unlock(bo->resv);
+	if (locked)
+		reservation_object_unlock(bo->resv);
 	kref_put(&bo->list_kref, ttm_bo_release_list);
 	return ret;
 }
--
2.7.4

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