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

[Umesh Nerlige Ramappa]

On Fri, Feb 10, 2023 at 09:20:25AM -0800, Umesh Nerlige Ramappa wrote:
>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.

My bad. The value would likely be 5. Anyways, it's just a nit.

>
>Thanks,
>Umesh
>
>>+
>>+	intel_perf_free(intel_perf);
>...