[Intel-xe] [PATCH] drm/xe: Fix error handling in xe_gt_record_default_lrcs

[Lucas De Marchi]

On Tue, Apr 04, 2023 at 07:59:19PM -0700, Matthew Brost wrote:
>Kill engines if an error is encountered in xe_gt_record_default_lrcs so
>the driver can free these resources on driver teardown.
>
>Signed-off-by: Matthew Brost <matthew.brost at intel.com>
>---
> drivers/gpu/drm/xe/xe_gt.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
>index bc821f431c45..3adde2205b39 100644
>--- a/drivers/gpu/drm/xe/xe_gt.c
>+++ b/drivers/gpu/drm/xe/xe_gt.c
>@@ -301,8 +301,12 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
>
> 		gt->default_lrc[hwe->class] = default_lrc;
> put_nop_e:
>+		if (err)
>+			xe_engine_kill(nop_e);
> 		xe_engine_put(nop_e);
> put_engine:
>+		if (err)
>+			xe_engine_kill(e);
> 		xe_engine_put(e);

these error handling here mixed with normal flow seem a little bit hard
to follow. I propose we change them as:

{
	struct xe_engine *e, *nop_e;
	struct xe_vm *vm;

	for_each_hw_engine(hwe, gt, id) {
		void *default_lrc;
		
		e = nop_e = NULL;

		vm = ...
		e = ...
		if (IS_ERR(e)) {
			...
			goto fail;
		}

		...

		xe_engine_put(nop_e);
                 xe_engine_put(e);
                 xe_vm_put(vm);
	}

	return 0;

fail:
	if (nop_e) {
		xe_engine_kill(nop_e);
		xe_engine_put(nop_e);
	}

	if (e) {
		xe_engine_kill(nop_e);
		xe_engine_put(nop_e);
	}
	
	xe_vm_put(vm);

	/* Wait for engine kill */
	msleep(50);

	return err;	
}

Although that msleep() seems arbitrary. Any alternative so we don't need
to keep tweaking the value for slower platforms? What exactly are we
waiting on?

Lucas De Marchi

> put_vm:
> 		xe_vm_put(vm);
>@@ -310,6 +314,10 @@ int xe_gt_record_default_lrcs(struct xe_gt *gt)
> 			break;
> 	}
>
>+	/* Wait for engine kill */
>+	if (err)
>+		msleep(50);
>+
> 	return err;
> }
>
>-- 
>2.34.1
>