Re: [PATCH v3 20/36] drm/xe/vf: Avoid indefinite blocking in preempt rebind worker for VFs supporting migration

["Lis, Tomasz" <tomasz.lis@intel.com>]

On 10/1/2025 3:45 PM, Lis, Tomasz wrote:
>
> On 9/29/2025 4:55 AM, Matthew Brost wrote:
>> Blocking in work queues on a hardware action that may never occur —
>> especially when it depends on a software fixup also scheduled on the
>> awork queue — is a recipe for deadlock. This situation arises with

Forgot about the typo - awork.

-Tomasz

>> the preempt rebind worker and VF post-migration recovery. To prevent
>> potential deadlocks, avoid indefinite blocking in the preempt rebind
>> worker for VFs that support migration.
>
> Some would say the timeout value is a magic number here, but I don't 
> have anything better to propose.
>
> And we do not have obligation to match each tracepoint _enter() with 
> _exit(), that's ok as well.
>
> So, all good:
>
> Reviewed-by: Tomasz Lis <tomasz.lis@intel.com>
>
>
>> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_vm.c | 29 ++++++++++++++++++++++++++++-
>>   1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index 80b7f13ecd80..b527ee2a5da5 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -35,6 +35,7 @@
>>   #include "xe_pt.h"
>>   #include "xe_pxp.h"
>>   #include "xe_res_cursor.h"
>> +#include "xe_sriov_vf.h"
>>   #include "xe_svm.h"
>>   #include "xe_sync.h"
>>   #include "xe_tile.h"
>> @@ -111,12 +112,25 @@ static int alloc_preempt_fences(struct xe_vm 
>> *vm, struct list_head *list,
>>   static int wait_for_existing_preempt_fences(struct xe_vm *vm)
>>   {
>>       struct xe_exec_queue *q;
>> +    bool vf_migration = IS_SRIOV_VF(vm->xe) &&
>> +        xe_sriov_vf_migration_supported(vm->xe);
>>         xe_vm_assert_held(vm);
>>         list_for_each_entry(q, &vm->preempt.exec_queues, lr.link) {
>>           if (q->lr.pfence) {
>> -            long timeout = dma_fence_wait(q->lr.pfence, false);
>> +            long timeout;
>> +
>> +            if (vf_migration)
>> +                timeout = dma_fence_wait_timeout(q->lr.pfence,
>> +                                 false, HZ / 5);
>> +            else
>> +                timeout = dma_fence_wait(q->lr.pfence, false);
>> +
>> +            if (!timeout) {
>> +                xe_assert(vm->xe, vf_migration);
>> +                return -EAGAIN;
>> +            }
>>                 /* Only -ETIME on fence indicates VM needs to be 
>> killed */
>>               if (timeout < 0 || q->lr.pfence->error == -ETIME)
>> @@ -541,6 +555,19 @@ static void preempt_rebind_work_func(struct 
>> work_struct *w)
>>   out_unlock_outer:
>>       if (err == -EAGAIN) {
>>           trace_xe_vm_rebind_worker_retry(vm);
>> +
>> +        /*
>> +         * We can't block in workers on a VF which supports migration
>> +         * given this can block the VF post-migration workers from
>> +         * getting scheduled.
>> +         */
>> +        if (IS_SRIOV_VF(vm->xe) &&
>> +            xe_sriov_vf_migration_supported(vm->xe)) {
>> +            up_write(&vm->lock);
>> +            xe_vm_queue_rebind_worker(vm);
>> +            return;
>> +        }
>> +
>>           goto retry;
>>       }