[PATCH 1/3] drm/xe/pf: Disable PF restart worker on device removal

[Michal Wajdeczko]

On 7/30/2025 11:08 PM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Michal Wajdeczko
> Sent: Wednesday, July 30, 2025 10:49 AM
> To: intel-xe at lists.freedesktop.org
> Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>
> Subject: [PATCH 1/3] drm/xe/pf: Disable PF restart worker on device removal
>>
>> We can't let restart worker run once device is removed, since other
>> data that it might want to access could be already released.
>> Explicitly disable worker as part of device cleanup action.
>>
>> Fixes: a4d1c5d0b99b ("drm/xe/pf: Move VFs reprovisioning to worker")
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt_sriov_pf.c | 30 ++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> index 35489fa81825..2761319fdc26 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf.c
>> @@ -50,6 +50,11 @@ static void pf_init_workers(struct xe_gt *gt)
>>  	INIT_WORK(&gt->sriov.pf.workers.restart, pf_worker_restart_func);
>>  }
>>  
>> +static void pf_fini_workers(struct xe_gt *gt)
>> +{
>> +	disable_work_sync(&gt->sriov.pf.workers.restart);
>> +}
>> +
>>  /**
>>   * xe_gt_sriov_pf_init_early - Prepare SR-IOV PF data structures on PF.
>>   * @gt: the &xe_gt to initialize
>> @@ -79,6 +84,21 @@ int xe_gt_sriov_pf_init_early(struct xe_gt *gt)
>>  	return 0;
>>  }
>>  
>> +static void pf_fini_action(void *arg)
>> +{
>> +	struct xe_gt *gt = arg;
>> +
>> +	pf_fini_workers(gt);
>> +}
>> +
>> +static int pf_init_late(struct xe_gt *gt)
>> +{
>> +	struct xe_device *xe = gt_to_xe(gt);
>> +
>> +	xe_gt_assert(gt, IS_SRIOV_PF(xe));
>> +	return devm_add_action_or_reset(xe->drm.dev, pf_fini_action, gt);
>> +}
>> +
>>  /**
>>   * xe_gt_sriov_pf_init - Prepare SR-IOV PF data structures on PF.
>>   * @gt: the &xe_gt to initialize
>> @@ -95,7 +115,15 @@ int xe_gt_sriov_pf_init(struct xe_gt *gt)
>>  	if (err)
>>  		return err;
>>  

[1]

>> -	return xe_gt_sriov_pf_migration_init(gt);
>> +	err = xe_gt_sriov_pf_migration_init(gt);
>> +	if (err)
>> +		return err;

[2]

>> +
>> +	err = pf_init_late(gt);
>> +	if (err)
>> +		return err;
> 
> Everything else looks okay, but I don't think this conditional branch is necessary.
> I think we can just return err here unconditionally, as if we fail this branch, err
> would equal zero anyway, which is what we'd want to return.

this was done on purpose

I'm pretty sure modern compilers can optimize that and
IMO it is clearer to leave success path fully visible

especially that in the future we might want to add more steps
to this init() function, so we could have smaller diff and
avoid diffs like [1-2] above

> 
> With that change:
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> -Jonathan Cavitt
> 
>> +
>> +	return 0;
>>  }
>>  
>>  static bool pf_needs_enable_ggtt_guest_update(struct xe_device *xe)
>> -- 
>> 2.47.1
>>
>>