Re: [PATCH i-g-t 2/2] tests/intel/perf_pmu: Migrate to unbind + unload

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Tue, Oct 22, 2024 at 10:05:02PM -0700, Lucas De Marchi wrote:
>There isn't much point in testing we can't remove the module directly
>after removing *some* known dependencies. The point in this test is to
>make sure that if the HW vanishes, driver still behaves. That is
>currently not true, as it will cause perf to explode.
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

LGTM,

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

Thanks,
Umesh
>---
> tests/intel/perf_pmu.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
>diff --git a/tests/intel/perf_pmu.c b/tests/intel/perf_pmu.c
>index bfa2d501a..6eb4f09d1 100644
>--- a/tests/intel/perf_pmu.c
>+++ b/tests/intel/perf_pmu.c
>@@ -2282,7 +2282,6 @@ static void test_unload(unsigned int num_engines)
> 		int fd[4 + num_engines * 3], i;
> 		uint64_t *buf;
> 		int count = 0, ret;
>-		char *who = NULL;
> 		int i915;
>
> 		i915 = __drm_open_driver(DRIVER_INTEL);
>@@ -2337,23 +2336,26 @@ static void test_unload(unsigned int num_engines)
>
> 		igt_debug("Read %d events from perf and trial unload\n", count);
> 		pmu_read_multi(fd[0], count, buf);
>-		ret = __igt_i915_driver_unload(&who);
>-		igt_debug("__igt_i915_driver_unload: ret %d who %s\n", ret, who);
>-		igt_assert(ret != 0 && !strcmp(who, "i915"));
>-		free(who);
>-		pmu_read_multi(fd[0], count, buf);
>
>-		igt_debug("Close perf\n");
>+		/*
>+		 * We can't unload the module with perf event registered, but
>+		 * we can make the HW go away by unbinding it from the driver.
>+		 */
>+		ret = igt_kmod_unbind("i915");
>+		igt_assert_eq(ret, 0);
>
>+		igt_debug("Close perf\n");
> 		for (i = 0; i < count; i++)
> 			close(fd[i]);
>
>+		igt_debug("Final unload\n");
>+		/* After perf is closed, we should be able to remove the module */
>+		ret = igt_i915_driver_unload();
>+		igt_assert_eq(ret, 0);
>+
> 		free(buf);
> 	}
> 	igt_waitchildren();
>-
>-	igt_debug("Final unload\n");
>-	igt_assert_eq(__igt_i915_driver_unload(NULL), 0);
> }
>
> static void pmu_read(int i915)
>-- 
>2.47.0
>