Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
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
>>

  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