Re: [PATCH] drm/amdkfd: Fix TCP XNACK scoreboard reset race

[Gang Ba <gaba@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]


On 9/1/26 12:45, Jay Cornwall wrote:
> On 9/1/2026 08:04, Russell, Kent wrote:
>
>>> +     .hqd_gfx_cleaner = kgd_gfx_v9_cleaner
>
> Prefer a name like fault_cleaner. This is not related to a HQD.
>
will update


>>> +void kgd_gfx_v9_cleaner(struct amdgpu_device *adev)
>>> +{
>>> +     gfx_v9_4_2_shader_cleaner_workarounds(adev);
>
> Similarly, fault_cleaner.
>
will update
>>> +static void gfx942_workaround_cleaner(struct kfd_node *dev, 
>>> uint16_t pasid)
>
> Similarly, fault_cleaner
>
will update

>>> +     /* Run the GFX cleaner workaround on the faulting device.
>>> +      * Only run once per page fault (evicted == 1 after first evict).
>>> +      */
>>> +     pdd = kfd_get_process_device_data(dev, p);
>>> +     if (pdd) {
>>> +             /* Only call cleaner once per page fault. Run the 
>>> workaround
>>> +              * only on the first eviction (evicted == 1 after 
>>> evict above).
>>> +              */
>>> +             dqm = pdd->dev->dqm;
>>> +             dqm_lock(dqm);
>>> +             if (pdd->qpd.evicted == 1 && dqm->dev->kfd2kgd-
>>>> hqd_gfx_cleaner)
>>> + dqm->dev->kfd2kgd->hqd_gfx_cleaner(dqm->dev-
>>>> adev);
>>> +             dqm_unlock(dqm);
>>> +     }
>>> +
>>> +     /* Restore all process devices after workaround completes */
>
> I'm not sure that we want to do this. Once a process faults there is 
> no recovery path for its queues. If we restore them then other waves 
> may cause further faults, leading us back here.
>
> Similarly, if the workqueue contains pipelined fault interrupts we 
> don't want to do this multiple times. Perhaps check pdd->qpd.evicted 
> before evicting queues and only run the workaround if (evicted == 0).
The eviction and restoration functions feature built-in concurrency 
protection. If an eviction process is already underway, the function 
automatically bypasses the operation.

The function utilizes the "evicted" value (which can be updated to 
|pdd->qpd.evicted <= 1|) to prevent multiple concurrent executions of 
the cleaner function.


[-- Attachment #2: Type: text/html, Size: 5303 bytes --]