[PATCH] amdgpu: Fix always_valid bos multiple LRU insertions.

[PATCH] amdgpu: Fix always_valid bos multiple LRU insertions.

[Bas Nieuwenhuizen]

If these bos are evicted and are in the validated list
things blow up, so do not put them in there. Notably,
that tries to add the bo to the LRU twice, which results
in a BUG_ON in ttm_bo.c.

While for the bo_list an alternative would be to not allow
always valid bos in there, that does not work for the user
fence.

Signed-off-by: Bas Nieuwenhuizen <basni@chromium.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 59089e027f4d8..95839d33af01d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -233,8 +233,10 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
 	for (i = 0; i < list->num_entries; i++) {
 		unsigned priority = list->array[i].priority;
 
-		list_add_tail(&list->array[i].tv.head,
-			      &bucket[priority]);
+		if (!list->array[i].robj->parent)
+			list_add_tail(&list->array[i].tv.head,
+			              &bucket[priority]);
+
 		list->array[i].user_pages = NULL;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f7fceb63413c9..cb3044258b352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -528,7 +528,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	INIT_LIST_HEAD(&duplicates);
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
 
-	if (p->uf_entry.robj)
+	if (p->uf_entry.robj && !p->uf_entry.robj->parent)
 		list_add(&p->uf_entry.tv.head, &p->validated);
 
 	while (1) {
-- 
2.16.0.rc1.238.g530d649a79-goog

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

Re: [PATCH] amdgpu: Fix always_valid bos multiple LRU insertions.

[Christian König]

Am 31.01.2018 um 13:07 schrieb Bas Nieuwenhuizen:
> If these bos are evicted and are in the validated list
> things blow up, so do not put them in there. Notably,
> that tries to add the bo to the LRU twice, which results
> in a BUG_ON in ttm_bo.c.
>
> While for the bo_list an alternative would be to not allow
> always valid bos in there, that does not work for the user
> fence.

The subject line should be "drm/amdgpu: ....", but apart from that the 
patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

>
> Signed-off-by: Bas Nieuwenhuizen <basni@chromium.org>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 ++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      | 2 +-
>   2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 59089e027f4d8..95839d33af01d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -233,8 +233,10 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
>   	for (i = 0; i < list->num_entries; i++) {
>   		unsigned priority = list->array[i].priority;
>   
> -		list_add_tail(&list->array[i].tv.head,
> -			      &bucket[priority]);
> +		if (!list->array[i].robj->parent)
> +			list_add_tail(&list->array[i].tv.head,
> +			              &bucket[priority]);
> +
>   		list->array[i].user_pages = NULL;
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index f7fceb63413c9..cb3044258b352 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -528,7 +528,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>   	INIT_LIST_HEAD(&duplicates);
>   	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
>   
> -	if (p->uf_entry.robj)
> +	if (p->uf_entry.robj && !p->uf_entry.robj->parent)
>   		list_add(&p->uf_entry.tv.head, &p->validated);
>   
>   	while (1) {

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

[PATCH] drm/amdgpu: Fix always_valid bos multiple LRU insertions.

[Bas Nieuwenhuizen]

If these bos are evicted and are in the validated list
things blow up, so do not put them in there. Notably,
that tries to add the bo to the LRU twice, which results
in a BUG_ON in ttm_bo.c.

While for the bo_list an alternative would be to not allow
always valid bos in there, that does not work for the user
fence.

Signed-off-by: Bas Nieuwenhuizen <basni@chromium.org>
Reviewed-by: Christian König <christian.koenig@amd.com>
---

Updated version for pushing, as I had the title prefix wrong as Christian pointed out.


 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 59089e027f4d8..95839d33af01d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -233,8 +233,10 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
 	for (i = 0; i < list->num_entries; i++) {
 		unsigned priority = list->array[i].priority;
 
-		list_add_tail(&list->array[i].tv.head,
-			      &bucket[priority]);
+		if (!list->array[i].robj->parent)
+			list_add_tail(&list->array[i].tv.head,
+			              &bucket[priority]);
+
 		list->array[i].user_pages = NULL;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f7fceb63413c9..cb3044258b352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -528,7 +528,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	INIT_LIST_HEAD(&duplicates);
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
 
-	if (p->uf_entry.robj)
+	if (p->uf_entry.robj && !p->uf_entry.robj->parent)
 		list_add(&p->uf_entry.tv.head, &p->validated);
 
 	while (1) {
-- 
2.16.0.rc1.238.g530d649a79-goog

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

[PATCH] drm/amdgpu: Fix always_valid bos multiple LRU insertions.

[Bas Nieuwenhuizen]

If these bos are evicted and are in the validated list
things blow up, so do not put them in there. Notably,
that tries to add the bo to the LRU twice, which results
in a BUG_ON in ttm_bo.c.

While for the bo_list an alternative would be to not allow
always valid bos in there, that does not work for the user
fence.

v2: Fixed whitespace issue pointed out by checkpatch.pl

Signed-off-by: Bas Nieuwenhuizen <basni@chromium.org>
Reviewed-by: Christian König <christian.koenig@amd.com>
---

Minor whitespace fix, sorry for the noise ...
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 59089e027f4d8..95839d33af01d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -233,8 +233,10 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
 	for (i = 0; i < list->num_entries; i++) {
 		unsigned priority = list->array[i].priority;
 
-		list_add_tail(&list->array[i].tv.head,
-			      &bucket[priority]);
+		if (!list->array[i].robj->parent)
+			list_add_tail(&list->array[i].tv.head,
+				      &bucket[priority]);
+
 		list->array[i].user_pages = NULL;
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f7fceb63413c9..cb3044258b352 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -528,7 +528,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	INIT_LIST_HEAD(&duplicates);
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
 
-	if (p->uf_entry.robj)
+	if (p->uf_entry.robj && !p->uf_entry.robj->parent)
 		list_add(&p->uf_entry.tv.head, &p->validated);
 
 	while (1) {
-- 
2.16.0.rc1.238.g530d649a79-goog

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

Re: [PATCH] drm/amdgpu: Fix always_valid bos multiple LRU insertions.

[Alex Deucher]

On Wed, Jan 31, 2018 at 7:58 AM, Bas Nieuwenhuizen <basni@chromium.org> wrote:
> If these bos are evicted and are in the validated list
> things blow up, so do not put them in there. Notably,
> that tries to add the bo to the LRU twice, which results
> in a BUG_ON in ttm_bo.c.
>
> While for the bo_list an alternative would be to not allow
> always valid bos in there, that does not work for the user
> fence.
>
> v2: Fixed whitespace issue pointed out by checkpatch.pl
>
> Signed-off-by: Bas Nieuwenhuizen <basni@chromium.org>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Applied.  thanks!

Alex

> ---
>
> Minor whitespace fix, sorry for the noise ...
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 6 ++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 59089e027f4d8..95839d33af01d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -233,8 +233,10 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
>         for (i = 0; i < list->num_entries; i++) {
>                 unsigned priority = list->array[i].priority;
>
> -               list_add_tail(&list->array[i].tv.head,
> -                             &bucket[priority]);
> +               if (!list->array[i].robj->parent)
> +                       list_add_tail(&list->array[i].tv.head,
> +                                     &bucket[priority]);
> +
>                 list->array[i].user_pages = NULL;
>         }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index f7fceb63413c9..cb3044258b352 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -528,7 +528,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>         INIT_LIST_HEAD(&duplicates);
>         amdgpu_vm_get_pd_bo(&fpriv->vm, &p->validated, &p->vm_pd);
>
> -       if (p->uf_entry.robj)
> +       if (p->uf_entry.robj && !p->uf_entry.robj->parent)
>                 list_add(&p->uf_entry.tv.head, &p->validated);
>
>         while (1) {
> --
> 2.16.0.rc1.238.g530d649a79-goog
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx