From: "Randhawa, Jagmeet" <jagmeet.randhawa@intel.com>
To: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Cc: <igt-dev@lists.freedesktop.org>
Subject: Re: [PATCH i-g-t v4 2/2] tests/intel/xe_exec_threads: Add multi-queue CGP_SYNC replay subtests
Date: Thu, 5 Nov 2026 10:42:00 -0800 [thread overview]
Message-ID: <45935f67-9b5e-4638-9d9b-07bf67fe712d@intel.com> (raw)
In-Reply-To: <auvqSoR7W7bi8vAW@nvishwa1-desk>
On 11/4/2026 6:30 PM, Niranjana Vishwanathapura wrote:
> On Wed, Nov 04, 2026 at 08:11:19PM +0000, Jagmeet Randhawa wrote:
>> Add coverage for the replay paths introduced by "drm/xe/multi_queue:
>> Handle
>> lost message during VF migration":
>> https://patchwork.freedesktop.org/series/165324/
>>
>> A CGP_SYNC message can be lost during VF migration and is replayed by
>> the
>> KMD on recovery. Exercising that needs a CGP_SYNC queued or
>> outstanding on
>> a live multi-queue group when the migration lands.
>>
>> A group serialises on one outstanding sync, so both CGP sources are
>> needed:
>> queue creation for registration updates, MULTI_QUEUE_PRIORITY for
>> dynamic
>> ones.
>>
>> A group holds at most MULTI_QUEUE_GROUP_SIZE queues, so asking for more
>> queues than that spans several groups and registers them in parallel.
>> The
>> legacy mode queue arrays are sized to the request rather than to a fixed
>> maximum, so the larger counts cost nothing for the tests that do not ask
>> for them.
>>
>> Add MULTI_QUEUE_PRIORITY to issue a short burst of priority updates
>> after
>> each exec, alternating the values since the KMD ignores a set to the
>> current value.
>>
>> Add two subtests built from these sources, repeated under
>> igt_until_timeout() with a bounded wall clock:
>>
>> multi-queue-many-groups registers several full groups in parallel
>> from multiple workers per engine
>> multi-queue-groups-priority drives dynamic updates on those groups
>> from
>> a single worker per engine
>>
>> The two cannot be merged. Driving dynamic updates alongside many
>> parallel
>> registrations makes an update wait long enough to exceed the KMD sync
>> timeout, which bans the group instead of leaving the update to be
>> replayed.
>>
>> v4:
>> - Size the legacy mode queue arrays to n_exec_queues instead of raising
>> MAX_N_EXEC_QUEUES, which would have grown the stack arrays of every
>> other test.
>> - Keep the multi-queue extension at its only use site.
>> - Mask instead of dividing in multi_queue_group_value().
>>
>> v3:
>> - Use the MULTI_QUEUE prefix.
>> - Drop the many groups flag and derive the group layout from the queue
>> count, since a group is limited to MULTI_QUEUE_GROUP_SIZE queues.
>> - Take the worker and queue counts from the section rather than deriving
>> them from flags, on top of the preceding patch.
>> - Four subtests down to two. Neither remaining subtest destroys
>> queues, so
>> the churn flag and the queue recreate path are dropped entirely rather
>> than renamed: registration traffic comes from the repeated runs under
>> igt_until_timeout().
>>
>> v2:
>> - Drop the secondary churn. A secondary cannot be replaced within a live
>> group, so MANY_QUEUES is no longer combined with MULTI_QUEUE and
>> MQ_CHURN_INTERVAL is gone.
>>
>> Signed-off-by: Jagmeet Randhawa <jagmeet.randhawa@intel.com>
>> ---
>> tests/intel/xe_exec_threads.c | 88 +++++++++++++++++++++++++++++++----
>> 1 file changed, 80 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/intel/xe_exec_threads.c
>> b/tests/intel/xe_exec_threads.c
>> index e5056192b..393ab8d75 100644
>> --- a/tests/intel/xe_exec_threads.c
>> +++ b/tests/intel/xe_exec_threads.c
>> @@ -50,6 +50,30 @@
>> #define MANY_QUEUES (0x1 << 14)
>> #define MULTI_QUEUE (0x1 << 15)
>> #define WQ_STRESS (0x1 << 16)
>> +#define MULTI_QUEUE_PRIORITY (0x1 << 17)
>> +
>> +/* Mirrors XE_MAX_GROUP_SIZE: one primary plus its secondaries. */
>> +#define MULTI_QUEUE_GROUP_SIZE 64
>> +
>> +/* Enough queues to span several full multi-queue groups. */
>> +#define N_EXEC_QUEUE_MULTI_GROUP (8 * MULTI_QUEUE_GROUP_SIZE)
>> +
>> +/*
>> + * Multi-queue priority updates issued per exec. The values must
>> alternate:
>> + * the KMD ignores a priority set to its current value, so a
>> constant value
>> + * would generate no CGP_SYNC traffic at all.
>> + */
>> +#define MULTI_QUEUE_PRIORITY_BURST 8
>> +
>> +/*
>> + * Workers per engine for the registration subtest. Registering several
>> + * groups at once widens the window where one queue's CGP_SYNC is still
>> + * outstanding while another queue waits on it.
>> + */
>> +#define MULTI_QUEUE_REG_THREADS 8
>
> Though we are using it for validating registration time cgp sync drops,
> the test itself just creates this many number threads per engine.
> Probably better to rename it as MULTI_QUEUE_THREADS_PER_ENGINE.
Renamed.
>
>> +
>> +/* Wall clock budget for the CGP_SYNC replay subtests. */
>> +#define CGP_REPEAT_SEC 5
>>
>> /*
>> * Maximum fence wait time when WQ_STRESS is active. If any bind/unbind
>> @@ -611,6 +635,18 @@ test_compute_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>> }
>> }
>>
>> +/*
>> + * Group membership for the queue at @i. A group holds at most
>> + * MULTI_QUEUE_GROUP_SIZE queues, so a larger queue count simply spans
>> + * several groups.
>> + */
>> +static uint64_t multi_queue_group_value(uint32_t *exec_queues, int i)
>> +{
>> + return i % MULTI_QUEUE_GROUP_SIZE ?
>> + exec_queues[i & ~(MULTI_QUEUE_GROUP_SIZE - 1)] :
>> + DRM_XE_MULTI_GROUP_CREATE;
>> +}
>> +
>> static void
>> test_legacy_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
>> struct drm_xe_engine_class_instance *eci, int n_exec_queues,
>> @@ -620,15 +656,13 @@ test_legacy_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>> { .type = DRM_XE_SYNC_TYPE_SYNCOBJ, .flags =
>> DRM_XE_SYNC_FLAG_SIGNAL, },
>> { .type = DRM_XE_SYNC_TYPE_SYNCOBJ, .flags =
>> DRM_XE_SYNC_FLAG_SIGNAL, },
>> };
>> - struct drm_xe_sync sync_all[MAX_N_EXEC_QUEUES];
>> + struct drm_xe_sync *sync_all;
>> struct drm_xe_exec exec = {
>> .num_batch_buffer = 1,
>> .num_syncs = 2,
>> .syncs = to_user_pointer(sync),
>> };
>> - uint32_t exec_queues[MAX_N_EXEC_QUEUES] = {};
>> - uint32_t bind_exec_queues[MAX_N_EXEC_QUEUES];
>> - uint32_t syncobjs[MAX_N_EXEC_QUEUES];
>> + uint32_t *exec_queues, *bind_exec_queues, *syncobjs;
>> size_t bo_size;
>> uint32_t bo = 0;
>> struct {
>> @@ -641,7 +675,11 @@ test_legacy_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>> int i, j, b, hang_exec_queue = n_exec_queues / 2;
>> bool owns_vm = false, owns_fd = false;
>>
>> - igt_assert_lte(n_exec_queues, MAX_N_EXEC_QUEUES);
>> + sync_all = calloc(n_exec_queues, sizeof(*sync_all));
>> + exec_queues = calloc(n_exec_queues, sizeof(*exec_queues));
>> + bind_exec_queues = calloc(n_exec_queues,
>> sizeof(*bind_exec_queues));
>> + syncobjs = calloc(n_exec_queues, sizeof(*syncobjs));
>> + igt_assert(sync_all && exec_queues && bind_exec_queues &&
>> syncobjs);
>
> This change is also not specific to MULTI_QUEUE and should be moved to
> the
> prep patch (patch #1 or separate prep patch). This patch should be
> multi-queue
> specific.
Moved to its own patch, the series is now 3 patches:
1/3 parametrise the worker and queue counts
2/3 allocate the exec queue arrays dynamically
3/3 the multi-queue subtests
>
> Also, I am seeing this change is not done in test_compute_mode. Looks
> like
> the new tests are not compute mode tests. We should make similar
> change in
> test_compute_mode() also and add compute mode/mixed mode tests also.
Done, patch 2/3 converts both test_legacy_mode() and test_compute_mode().
test_balancer() still uses fixed arrays and keeps the MAX_N_EXEC_QUEUES
check.
No balancer subtest asks for more than N_EXEC_QUEUE, and BALANCER cannot be
combined with the new subtests since patch 1/3 requires one worker per
engine
when BALANCER is set. Happy to convert it as well if you would rather
all three
were consistent.
>
>>
>> if (flags & FD) {
>> fd = drm_reopen_driver(fd);
>> @@ -676,7 +714,6 @@ test_legacy_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>> }
>> memset(data, 0, bo_size);
>>
>> - memset(sync_all, 0, sizeof(sync_all));
>> for (i = 0; i < n_exec_queues; i++) {
>> if (flags & MULTI_QUEUE) {
>> struct drm_xe_ext_set_property multi_queue = {
>> @@ -687,7 +724,7 @@ test_legacy_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>>
>> uint64_t ext = to_user_pointer(&multi_queue);
>>
>> - multi_queue.value = i ? exec_queues[0] :
>> DRM_XE_MULTI_GROUP_CREATE;
>> + multi_queue.value = multi_queue_group_value(exec_queues,
>> i);
>> exec_queues[i] = xe_exec_queue_create(fd, vm, eci, ext);
>> } else {
>> if (!(flags & MANY_QUEUES))
>> @@ -785,6 +822,16 @@ test_legacy_mode(int fd, uint32_t vm, uint64_t
>> addr, uint64_t userptr,
>> xe_exec(fd, &exec);
>> }
>>
>> + if (flags & MULTI_QUEUE_PRIORITY) {
>> + uint32_t prop =
>> + DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY;
>> + int z;
>> +
>> + for (z = 0; z < MULTI_QUEUE_PRIORITY_BURST; z++)
>> + xe_exec_queue_set_property(fd, exec_queues[e],
>> + prop, z & 1);
>> + }
>> +
>> if (flags & REBIND && i && !(i & 0x1f)) {
>> xe_vm_unbind_async(fd, vm, bind_exec_queues[e],
>> 0, addr, bo_size,
>> @@ -942,6 +989,11 @@ wq_stress_cleanup:
>> xe_vm_destroy(fd, vm);
>> if (owns_fd)
>> drm_close_driver(fd);
>> +
>> + free(sync_all);
>> + free(exec_queues);
>> + free(bind_exec_queues);
>> + free(syncobjs);
>> }
>>
>> struct thread_data {
>> @@ -1228,6 +1280,10 @@ static void *thread(void *data)
>> * arg[1]:
>> * @multi-queue-basic:
>> * multi-queue basic
>> + * @multi-queue-many-groups:
>> + * multi-queue many groups
>> + * @multi-queue-groups-priority:
>> + * multi-queue groups priority
>> * @multi-queue-userptr:
>> * multi-queue userptr
>> * @multi-queue-rebind:
>> @@ -1507,6 +1563,9 @@ int igt_main()
>> const struct section {
>> const char *name;
>> unsigned int flags;
>> + unsigned int repeat_secs;
>> + int n_threads_per_engine;
>> + int n_exec_queues;
>> } sections[] = {
>> { "basic", 0 },
>> { "many-queues", MANY_QUEUES },
>> @@ -1753,6 +1812,10 @@ int igt_main()
>> USERPTR | INVALIDATE },
>> { "multi-queue-cm-fd-userptr-invalidate-race", MULTI_QUEUE |
>> COMPUTE_MODE | FD |
>> USERPTR | INVALIDATE | RACE },
>> + { "multi-queue-many-groups", MULTI_QUEUE, CGP_REPEAT_SEC,
>> + MULTI_QUEUE_REG_THREADS, N_EXEC_QUEUE_MULTI_GROUP },
>> + { "multi-queue-groups-priority", MULTI_QUEUE |
>> MULTI_QUEUE_PRIORITY,
>> + CGP_REPEAT_SEC, 1, N_EXEC_QUEUE_MULTI_GROUP },
>
> As commented above, probably adding compute mode or mixed mode tests
> also here
> would be good. I would suggest probably adding one
> 'multi-queue-mixed-many-groups'
> would be good enough.
>
> Niranjana
Added.
This also needed test_compute_mode() to place queues the same way as
test_legacy_mode(). It put every queue in a single group, which exceeds the
group limit at these queue counts, so both paths now use
multi_queue_group_value() and the helper moved ahead of test_compute_mode().
On NVL-P the mixed subtest takes 10.5s and produces the same 16384
CGP_SYNCs as
the legacy only one.
Jagmeet
>
>> { NULL },
>> };
>> int fd;
>> @@ -1762,6 +1825,11 @@ int igt_main()
>>
>> for (const struct section *s = sections; s->name; s++) {
>> igt_subtest_f("threads-%s", s->name) {
>> + int n_threads = s->n_threads_per_engine ?
>> + s->n_threads_per_engine : 1;
>> + int n_queues = s->n_exec_queues ?
>> + s->n_exec_queues : N_EXEC_QUEUE;
>> +
>> if (s->flags & MULTI_QUEUE) {
>> igt_skip_on_f(!(xe_has_multi_queue_engine(fd)),
>> "multi_queue is supported on graphics version
>> 35 and above");
>> @@ -1772,7 +1840,11 @@ int igt_main()
>> if ((s->flags & MULTI_QUEUE) && (s->flags & HANG))
>> ignore_multi_queue_hang_warnings_in_dmesg();
>>
>> - threads(fd, s->flags, 1, N_EXEC_QUEUE);
>> + if (s->repeat_secs)
>> + igt_until_timeout(s->repeat_secs)
>> + threads(fd, s->flags, n_threads, n_queues);
>> + else
>> + threads(fd, s->flags, n_threads, n_queues);
>> }
>> }
>>
>> --
>> 2.53.0
>>
next prev parent reply other threads:[~2026-11-05 18:43 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 20:11 [PATCH i-g-t v4 0/2] Multi-queue CGP_SYNC replay coverage Jagmeet Randhawa
2026-11-04 20:11 ` [PATCH i-g-t v4 1/2] tests/intel/xe_exec_threads: Parametrise worker and queue counts Jagmeet Randhawa
2026-11-05 2:21 ` Niranjana Vishwanathapura
2026-11-05 18:37 ` Randhawa, Jagmeet
2026-11-04 20:11 ` [PATCH i-g-t v4 2/2] tests/intel/xe_exec_threads: Add multi-queue CGP_SYNC replay subtests Jagmeet Randhawa
2026-11-05 2:30 ` Niranjana Vishwanathapura
2026-11-05 18:42 ` Randhawa, Jagmeet [this message]
2026-11-04 21:10 ` ✓ Xe.CI.BAT: success for Multi-queue CGP_SYNC replay coverage Patchwork
2026-11-04 21:18 ` ✓ i915.CI.BAT: " Patchwork
2026-11-05 6:02 ` ✓ Xe.CI.BAT: success for Multi-queue CGP_SYNC replay coverage (rev2) Patchwork
2026-11-05 6:19 ` ✓ i915.CI.BAT: " Patchwork
2026-11-05 10:46 ` ✗ Xe.CI.FULL: failure for Multi-queue CGP_SYNC replay coverage Patchwork
2026-11-05 21:29 ` ✗ Xe.CI.FULL: failure for Multi-queue CGP_SYNC replay coverage (rev2) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45935f67-9b5e-4638-9d9b-07bf67fe712d@intel.com \
--to=jagmeet.randhawa@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=niranjana.vishwanathapura@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox