On 2018-11-12 12:09 p.m., Christian König wrote:
> We accidentially set the huge flag on the parent instead of the childs.
> This caused some VM faults under memory pressure.

Reviewed-by: Felix Kuehling <felix.kuehl...@amd.com>

I got a bit confused when re-reading this code. Maybe part of it is that
cursor.entry is not really an entry (as in PTE), but a page table.
cursor.pfn identifies the entry within the table. Maybe renaming
cursor.entry to something like cursor.table would make it slightly more
intuitive.

Regards,
  Felix


>
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 352b30409060..dad0e2342df9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1632,13 +1632,6 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>                       continue;
>               }
>  
> -             /* First check if the entry is already handled */
> -             if (cursor.pfn < frag_start) {
> -                     cursor.entry->huge = true;
> -                     amdgpu_vm_pt_next(adev, &cursor);
> -                     continue;
> -             }
> -
>               /* If it isn't already handled it can't be a huge page */
>               if (cursor.entry->huge) {
>                       /* Add the entry to the relocated list to update it. */
> @@ -1701,8 +1694,17 @@ static int amdgpu_vm_update_ptes(struct 
> amdgpu_pte_update_params *params,
>                       }
>               } while (frag_start < entry_end);
>  
> -             if (frag >= shift)
> +             if (amdgpu_vm_pt_descendant(adev, &cursor)) {
> +                     /* Mark all child entries as huge */
> +                     while (cursor.pfn < frag_start) {
> +                             cursor.entry->huge = true;
> +                             amdgpu_vm_pt_next(adev, &cursor);
> +                     }
> +
> +             } else if (frag >= shift) {
> +                     /* or just move on to the next on the same level. */
>                       amdgpu_vm_pt_next(adev, &cursor);
> +             }
>       }
>  
>       return 0;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to