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 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 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.