All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.