[igt-dev] [PATCH i-g-t v2 1/1] tests/gem_ctx_exec: Exercise barrier race

[Umesh Nerlige Ramappa]

On Thu, Feb 09, 2023 at 12:50:39PM +0100, Janusz Krzysztofik wrote:
>Users reported oopses on list corruptions when using i915 perf with a
>number of concurrently running graphics applications.  That indicates we
>are currently missing some important tests for such scenarios.  Cover
>that gap.
>
>Since root cause analysis pointed out to an issue in barrier processing
>code, add a new subtest focused on exercising interaction between perf
>open / close operations, which replace active barriers with perf requests
>on kernel contexts, and concurrent barrier preallocate / acquire
>operations performed during context first pin / last unpin.
>
>v2: convert to a separate subtest, not a variant of another one (that has
>    been dropped from the series),
>  - move the subtest out of tests/i915/perf.c (Ashutosh), add it to
>    tests/i915/gem_ctx_exec.c,
>  - don't touch lib/i915/perf.c (Umesh, Ashutosh), duplicate reused code
>    from tests/i915/perf.c in tests/i915/gem_ctx_exec.c.
>
>References: https://gitlab.freedesktop.org/drm/intel/-/issues/6333
>Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
>---
> tests/i915/gem_ctx_exec.c | 123 ++++++++++++++++++++++++++++++++++++++
> tests/meson.build         |   9 ++-
> 2 files changed, 131 insertions(+), 1 deletion(-)
>
>diff --git a/tests/i915/gem_ctx_exec.c b/tests/i915/gem_ctx_exec.c
>index 3d94f01db9..97fbc50e97 100644
>--- a/tests/i915/gem_ctx_exec.c
>+++ b/tests/i915/gem_ctx_exec.c
>@@ -42,6 +42,7 @@
>
> #include "i915/gem.h"
> #include "i915/gem_create.h"
>+#include "i915/perf.h"
> #include "igt.h"
> #include "igt_dummyload.h"
> #include "igt_rand.h"
>@@ -448,6 +449,115 @@ static void close_race(int i915)
> 	munmap(ctx_id, 4096);
> }
>
>+static uint64_t timebase_scale(struct intel_perf *intel_perf, uint32_t u32_delta)
>+{
>+	return ((uint64_t)u32_delta * NSEC_PER_SEC) / intel_perf->devinfo.timestamp_frequency;
>+}
>+
>+/* Returns: the largest OA exponent that will still result in a sampling period
>+ * less than or equal to the given @period.
>+ */
>+static int max_oa_exponent_for_period_lte(struct intel_perf *intel_perf, uint64_t period)
>+{
>+	/* NB: timebase_scale() takes a uint32_t and an exponent of 30
>+	 * would already represent a period of ~3 minutes so there's
>+	 * really no need to consider higher exponents.
>+	 */
>+	for (int i = 0; i < 30; i++) {
>+		uint64_t oa_period = timebase_scale(intel_perf, 2 << i);
>+
>+		if (oa_period > period)
>+			return max(0, i - 1);
>+	}
>+
>+	igt_assert(!"reached");
>+	return -1;
>+}
>+
>+static void perf_open_close_loop(int fd, int *done)
>+{
>+	struct intel_perf_metric_set *metric_set = NULL, *metric_set_iter;
>+	struct intel_perf *intel_perf = intel_perf_for_fd(fd);
>+	uint64_t properties[] = {
>+		DRM_I915_PERF_PROP_SAMPLE_OA, true,
>+		DRM_I915_PERF_PROP_OA_METRICS_SET, 0,
>+		DRM_I915_PERF_PROP_OA_FORMAT, 0,
>+		DRM_I915_PERF_PROP_OA_EXPONENT, 0,
>+	};
>+	struct drm_i915_perf_open_param param = {
>+		.flags = I915_PERF_FLAG_FD_CLOEXEC,

nit: If you also add I915_PERF_FLAG_DISABLED here, then OA buffer 
reports will be disabled. This will make sure that the perf API does not 
enable the OA unit. It will still run the code that you are targeting.

>+		.num_properties = sizeof(properties) / 16,
>+		.properties_ptr = to_user_pointer(properties),
>+	};
>+	uint32_t devid = intel_get_drm_devid(fd);
>+
>+	igt_require(intel_perf);
>+	intel_perf_load_perf_configs(intel_perf, fd);
>+
>+	igt_require(devid);
>+	igt_list_for_each_entry(metric_set_iter, &intel_perf->metric_sets, link) {
>+		if (!strcmp(metric_set_iter->symbol_name,
>+			    IS_HASWELL(devid) ? "RenderBasic" : "TestOa")) {
>+			metric_set = metric_set_iter;
>+			break;
>+		}
>+	}
>+	igt_require(metric_set);
>+	igt_require(metric_set->perf_oa_metrics_set);
>+	properties[3] = metric_set->perf_oa_metrics_set;
>+	properties[5] = metric_set->perf_oa_format;
>+
>+	igt_require(intel_perf->devinfo.timestamp_frequency);
>+	properties[7] = max_oa_exponent_for_period_lte(intel_perf, 1000);

nit: The result of max_oa_exponent_for_period_lte() can be hard coded 
here (likely 1 << 5) and you could remove the additional code added for 
max_oa_exponent_for_period_lte(). This parameter only controls the 
frequency at which the OA reports are captured into the OA buffer and it 
has no relation to the barrier related code in perf.

Thanks,
Umesh

>+
>+	intel_perf_free(intel_perf);
...