Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Unbounded VMA prefaulting loop causes CPU starvation and soft lockups.
--

commit 359edbd46613b71dc869a156d98f9fb5a002a599
Author: Arvind Yadav <[email protected]>

drm/xe: Reuse one dummy page per BO after wedge

This commit replaces the allocation of a dummy page on every call to
ttm_bo_vm_dummy_page() with a single zeroed page kept per BO. It maps the
faulting address first and then prefaults the rest of the VMA as a best-effort
optimization.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index b86cd6030ed6b..1f6ea9f5afe62 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
[ ... ]
> @@ -2080,6 +2083,50 @@ static vm_fault_t xe_bo_cpu_fault_fastpath(struct 
> vm_fault *vmf, struct xe_devic
>       return ret;
>  }
>  
> +static vm_fault_t xe_bo_vm_dummy_page(struct vm_fault *vmf, struct xe_bo *bo)
> +{
> +     struct vm_area_struct *vma = vmf->vma;
[ ... ]
> +     /* The faulting address must be mapped successfully. */
> +     ret = vmf_insert_pfn_prot(vma, vmf->address, pfn,
> +                               vma->vm_page_prot);
> +     if (ret & VM_FAULT_ERROR)
> +             return ret;
> +
> +     /* Prefault the remaining VMA as a best-effort optimization. */
> +     for (address = vma->vm_start; address < vma->vm_end;
> +          address += PAGE_SIZE) {

[Severity: High]
This is a pre-existing issue, but does this unbounded loop risk causing CPU
starvation and soft lockups?

When a user allocates and mmaps a very large BO, and the device becomes
wedged, a page fault will call xe_bo_cpu_fault() and trigger this path in
xe_bo_vm_dummy_page(). For huge buffer objects (e.g., tens of gigabytes),
iterating millions of times to insert PTEs without yielding points or signal
checks can cause severe CPU starvation and watchdog soft lockups. This can
also make the faulting task temporarily unkillable while stalling concurrent
mmap_write_lock() requests.

This behavior was inherited from ttm_bo_vm_dummy_page(), but should this
loop be bounded or conditionally yield?

> +             if (address == vmf->address)
> +                     continue;
> +
> +             prefault_ret = vmf_insert_pfn_prot(vma, address, pfn,
> +                                                vma->vm_page_prot);
> +             if (prefault_ret & VM_FAULT_ERROR)
> +                     break;
> +     }
> +
> +     return ret;
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=6

Reply via email to