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
>>
next prev parent 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