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 1/2] tests/intel/xe_exec_threads: Parametrise worker and queue counts
Date: Thu, 5 Nov 2026 10:37:48 -0800	[thread overview]
Message-ID: <3d48b710-e230-4152-84e6-e8da3001eb90@intel.com> (raw)
In-Reply-To: <auvoIUnk8NEJlCWj@nvishwa1-desk>


On 11/4/2026 6:21 PM, Niranjana Vishwanathapura wrote:
> On Wed, Nov 04, 2026 at 08:11:18PM +0000, Jagmeet Randhawa wrote:
>> threads() hardcodes one worker thread per engine and N_EXEC_QUEUE exec
>> queues per worker. Subtests that need a different shape have no way 
>> to ask
>> for one.
>>
>> Pass the number of worker threads per engine and the number of exec 
>> queues
>> per worker as arguments, with the existing callers keeping the current
>> behaviour by passing 1 and N_EXEC_QUEUE.
>>
>> The balancer threads are not created per engine, so require one 
>> worker per
>> engine when BALANCER is set.
>>
>> No functional change.
>>
>> v4:
>> - Actually use n_exec_queues for the per engine workers. The previous
>>   revision still set the fixed N_EXEC_QUEUE there, so the queue count
>>   never reached them.
>> - Nest the worker loop inside the engine walk, after the engines that
>>   cannot run the job are skipped.
>> - Assert one worker per engine when BALANCER is set.
>>
>> Signed-off-by: Jagmeet Randhawa <jagmeet.randhawa@intel.com>
>> ---
>> tests/intel/xe_exec_threads.c | 71 ++++++++++++++++++++---------------
>> 1 file changed, 41 insertions(+), 30 deletions(-)
>>
>> diff --git a/tests/intel/xe_exec_threads.c 
>> b/tests/intel/xe_exec_threads.c
>> index f03f20142..e5056192b 100644
>> --- a/tests/intel/xe_exec_threads.c
>> +++ b/tests/intel/xe_exec_threads.c
>> @@ -32,6 +32,7 @@
>> #include <string.h>
>>
>> #define MAX_N_EXEC_QUEUES    16
>> +#define N_EXEC_QUEUE        16
>> #define USERPTR        (0x1 << 0)
>> #define REBIND        (0x1 << 1)
>> #define INVALIDATE    (0x1 << 2)
>> @@ -1343,7 +1344,8 @@ static void *thread(void *data)
>>  *    multi-queue compute mode fd userptr invalidate race
>>  */
>>
>> -static void threads(int fd, int flags)
>> +static void threads(int fd, int flags, int n_threads_per_engine,
>> +            int n_exec_queues)
>> {
>>     struct thread_data *threads_data;
>>     struct drm_xe_engine_class_instance *hwe;
>> @@ -1351,13 +1353,16 @@ static void threads(int fd, int flags)
>>     uint64_t userptr = 0x00007000eadbe000;
>>     pthread_mutex_t mutex;
>>     pthread_cond_t cond;
>> -    int n_engines = 0, class;
>> +    int n_engines = 0, class, t;
>>     uint64_t i = 0;
>>     uint32_t vm_legacy_mode = 0, vm_compute_mode = 0;
>>     bool go = false;
>>     int n_threads = 0;
>>     int gt;
>>
>> +    /* The balancer threads below are not created per engine. */
>> +    igt_assert(!(flags & BALANCER) || n_threads_per_engine == 1);
>
> May be also add an assert for MULTI_QUEUE and MANY_QUEUES are also not
> set at the same time (not supported currently).
>
> Niranjana

Added next to the BALANCER assert in this patch.

Jagmeet

>
>> +
>>     xe_for_each_engine(fd, hwe) {
>>         if ((flags & MULTI_QUEUE) && 
>> !xe_engine_class_supports_multi_queue(fd, hwe->engine_class))
>>             continue;
>> @@ -1374,7 +1379,8 @@ static void threads(int fd, int flags)
>>             }
>>     }
>>
>> -    threads_data = calloc(n_engines, sizeof(*threads_data));
>> +    threads_data = calloc(n_engines * n_threads_per_engine,
>> +                  sizeof(*threads_data));
>>     igt_assert(threads_data);
>>
>>     pthread_mutex_init(&mutex, 0);
>> @@ -1388,33 +1394,37 @@ static void threads(int fd, int flags)
>>     }
>>
>>     xe_for_each_engine(fd, hwe) {
>> -        if ((flags & MULTI_QUEUE) && 
>> !xe_engine_class_supports_multi_queue(fd, hwe->engine_class))
>> +        if ((flags & MULTI_QUEUE) &&
>> +            !xe_engine_class_supports_multi_queue(fd,
>> +                              hwe->engine_class))
>>             continue;
>> -        threads_data[i].mutex = &mutex;
>> -        threads_data[i].cond = &cond;
>> +
>> +        for (t = 0; t < n_threads_per_engine; t++) {
>> +            threads_data[i].mutex = &mutex;
>> +            threads_data[i].cond = &cond;
>> #define ADDRESS_SHIFT    39
>> -        threads_data[i].addr = addr | (i << ADDRESS_SHIFT);
>> -        threads_data[i].userptr = userptr | (i << ADDRESS_SHIFT);
>> -        threads_data[i].fd = fd;
>> -        threads_data[i].vm_legacy_mode = vm_legacy_mode;
>> -        threads_data[i].vm_compute_mode = vm_compute_mode;
>> -        threads_data[i].eci = hwe;
>> -#define N_EXEC_QUEUE    16
>> -        threads_data[i].n_exec_queue = N_EXEC_QUEUE;
>> +            threads_data[i].addr = addr | (i << ADDRESS_SHIFT);
>> +            threads_data[i].userptr = userptr | (i << ADDRESS_SHIFT);
>> +            threads_data[i].fd = fd;
>> +            threads_data[i].vm_legacy_mode = vm_legacy_mode;
>> +            threads_data[i].vm_compute_mode = vm_compute_mode;
>> +            threads_data[i].eci = hwe;
>> +            threads_data[i].n_exec_queue = n_exec_queues;
>> #define N_EXEC        1024
>> -        threads_data[i].n_exec = N_EXEC;
>> -        threads_data[i].flags = flags;
>> -        if (flags & MIXED_MODE) {
>> -            threads_data[i].flags &= ~MIXED_MODE;
>> -            if (i & 1)
>> -                threads_data[i].flags |= COMPUTE_MODE;
>> -        }
>> -        threads_data[i].go = &go;
>> +            threads_data[i].n_exec = N_EXEC;
>> +            threads_data[i].flags = flags;
>> +            if (flags & MIXED_MODE) {
>> +                threads_data[i].flags &= ~MIXED_MODE;
>> +                if (i & 1)
>> +                    threads_data[i].flags |= COMPUTE_MODE;
>> +            }
>> +            threads_data[i].go = &go;
>>
>> -        ++n_threads;
>> -        pthread_create(&threads_data[i].thread, 0, thread,
>> -                   &threads_data[i]);
>> -        ++i;
>> +            ++n_threads;
>> +            pthread_create(&threads_data[i].thread, 0, thread,
>> +                       &threads_data[i]);
>> +            ++i;
>> +        }
>>     }
>>
>>     if (flags & BALANCER) {
>> @@ -1448,7 +1458,7 @@ static void threads(int fd, int flags)
>>                     threads_data[i].vm_legacy_mode =
>>                         vm_legacy_mode;
>>                     threads_data[i].class = class;
>> -                    threads_data[i].n_exec_queue = N_EXEC_QUEUE;
>> +                    threads_data[i].n_exec_queue = n_exec_queues;
>>                     threads_data[i].n_exec = N_EXEC;
>>                     threads_data[i].flags = flags;
>>                     threads_data[i].flags &= ~BALANCER;
>> @@ -1471,7 +1481,7 @@ static void threads(int fd, int flags)
>>     pthread_cond_broadcast(&cond);
>>     pthread_mutex_unlock(&mutex);
>>
>> -    for (i = 0; i < n_engines; ++i)
>> +    for (i = 0; i < n_threads; ++i)
>>         pthread_join(threads_data[i].thread, NULL);
>>
>>     if (vm_legacy_mode)
>> @@ -1762,7 +1772,7 @@ int igt_main()
>>             if ((s->flags & MULTI_QUEUE) && (s->flags & HANG))
>>                 ignore_multi_queue_hang_warnings_in_dmesg();
>>
>> -            threads(fd, s->flags);
>> +            threads(fd, s->flags, 1, N_EXEC_QUEUE);
>>         }
>>     }
>>
>> @@ -1863,7 +1873,8 @@ int igt_main()
>>             atomic_store(&wq_stress_hang_detected, false);
>>             igt_until_timeout(WQ_STRESS_DURATION_SEC) {
>>                 threads(child_fd,
>> -                    REBIND | BIND_EXEC_QUEUE | WQ_STRESS);
>> +                    REBIND | BIND_EXEC_QUEUE | WQ_STRESS,
>> +                    1, N_EXEC_QUEUE);
>>                 if (atomic_load(&wq_stress_hang_detected))
>>                     break;
>>             }
>> -- 
>> 2.53.0
>>

  reply	other threads:[~2026-11-05 18:38 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 [this message]
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
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=3d48b710-e230-4152-84e6-e8da3001eb90@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