Re: [PATCH 2/2] tests/intel/xe_exec_multi_queue: replace sleep with barrier queue

["Wang, X" <x.wang@intel.com>]

On 4/29/2026 11:27, Summers, Stuart wrote:
> On Tue, 2026-04-28 at 19:08 -0700, Niranjana Vishwanathapura wrote:
>> In __test_priority() DYN_PRIORITY case, replace sleep() with a
>> deterministic barrier using an extra queue in the same multi-queue
>> group. After assigning priorities, submit a spinner to the extra
>> queue, end it immediately and wait for its user fence to signal.
>> This guarantees a full scheduler round-trip confirming the priority
>> updates have taken effect before releasing the other queues.
>>
>> Increase exec_queues[] and spin[] array sizes by 1 to accommodate
>> the extra barrier queue slot at index num_queues.
>>
>> Assisted-by: GitHub Copilot:claude-sonnet-4.6
>> Signed-off-by: Niranjana Vishwanathapura
>> <niranjana.vishwanathapura@intel.com>
>> ---
>>   tests/intel/xe_exec_multi_queue.c | 37 +++++++++++++++++++++++++----
>> --
>>   1 file changed, 30 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/intel/xe_exec_multi_queue.c
>> b/tests/intel/xe_exec_multi_queue.c
>> index 382705d065..8c6fbb2d18 100644
>> --- a/tests/intel/xe_exec_multi_queue.c
>> +++ b/tests/intel/xe_exec_multi_queue.c
>> @@ -381,8 +381,8 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>                  .syncs = to_user_pointer(&sync),
>>          };
>>          uint64_t vm_sync = 0, addr = BASE_ADDRESS;
>> -       uint32_t exec_queues[XE_EXEC_QUEUE_PRIORITY_N];
>> -       struct xe_spin *spin[XE_EXEC_QUEUE_PRIORITY_N];
>> +       uint32_t exec_queues[XE_EXEC_QUEUE_PRIORITY_N + 1];
>> +       struct xe_spin *spin[XE_EXEC_QUEUE_PRIORITY_N + 1];
> Since we're only really making use of this in the dynamic case, should
> we have "+ !!DYNAMIC" instead of "+ 1" here? I.e. we only care about
> the extra barrier one in the dynamic case?
>
> Thanks,
> Stuart
flags is a runtime parameter, so + !!(flags & DYN_PRIORITY)
would make the array size runtime-determined — effectively a VLA.
Even in userspace, VLAs are generally discouraged due to
unpredictable stack usage. The cost of one extra slot is negligible,
so always using + 1 is simpler and avoids introducing a VLA.

Thanks,
Xin
>>          uint32_t vm, num_queues, num_queue_priorities, bo = 0;
>>          uint32_t start_order[XE_EXEC_QUEUE_PRIORITY_N] = { 0 };
>>          int64_t fence_timeout = NSEC_PER_SEC;
>> @@ -403,7 +403,7 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>                  .value = DRM_XE_MULTI_GROUP_CREATE,
>>          };
>>          uint64_t ext = to_user_pointer(&multi_queue);
>> -       int i, j, sleep_duration = 1;
>> +       int i, j;
>>          void *bo_map;
>>   
>>          num_queue_priorities = XE_EXEC_QUEUE_NUM_PRIORITIES;
>> @@ -415,12 +415,12 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>                   eci[0].engine_class, eci[0].engine_instance);
>>   
>>          vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_LR_MODE, 0);
>> -       bo_size = xe_bb_size(fd, sizeof(*spin[0]) * num_queues);
>> +       bo_size = xe_bb_size(fd, sizeof(*spin[0]) * (num_queues +
>> 1));
>>   
>>          bo = xe_bo_create(fd, vm, bo_size, vram_if_possible(fd,
>> eci[0].gt_id),
>>                            DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
>>          bo_map = xe_bo_map(fd, bo, bo_size);
>> -       for (i = 0; i < num_queues; i++)
>> +       for (i = 0; i < num_queues + 1; i++)
>>                  spin[i] = bo_map + i * sizeof(*spin[0]);
>>   
>>          /* Use the default priority for Q0 because we are explicitly
>> waiting for it below */
>> @@ -430,6 +430,11 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>          if (flags & DYN_PRIORITY) {
>>                  for (i = 1; i < num_queues; i++)
>>                          exec_queues[i] = xe_exec_queue_create(fd, vm,
>> eci, ext);
>> +               /*
>> +                * Create an extra queue in the same multi-queue
>> group, used as
>> +                * a barrier to confirm priority updates have taken
>> effect.
>> +                */
>> +               exec_queues[num_queues] = xe_exec_queue_create(fd,
>> vm, eci, ext);
>>          } else {
>>                  struct drm_xe_ext_set_property mq_priority = {
>>                          .base.name =
>> DRM_XE_EXEC_QUEUE_EXTENSION_SET_PROPERTY,
>> @@ -474,14 +479,28 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>                  xe_spin_wait_started(spin[i]);
>>   
>>          if (flags & DYN_PRIORITY) {
>> +               uint64_t barrier_spin_addr = addr + num_queues *
>> sizeof(struct xe_spin);
>> +
>>                  /* Assign increasing order of priority for secondary
>> queues */
>>                  for (i = 1; i < num_queues; i++)
>>                          xe_exec_queue_set_property(fd,
>> exec_queues[i],
>>                                                    
>> DRM_XE_EXEC_QUEUE_SET_PROPERTY_MULTI_QUEUE_PRIORITY,
>>                                                     i %
>> num_queue_priorities);
>>   
>> -               /* Wait for priorities to take effect */
>> -               sleep(sleep_duration);
>> +               /*
>> +                * Submit a barrier job on the extra queue to ensure
>> priority
>> +                * updates have taken effect before releasing the
>> other queues.
>> +                */
>> +               xe_spin_init_opts(spin[num_queues], .addr =
>> barrier_spin_addr,
>> +                                 .preempt = true);
>> +               sync.addr = barrier_spin_addr +
>> +                       ((char *)&spin[num_queues]->exec_sync - (char
>> *)spin[num_queues]);
>> +               exec.exec_queue_id = exec_queues[num_queues];
>> +               exec.address = barrier_spin_addr;
>> +               xe_exec(fd, &exec);
>> +               xe_spin_end(spin[num_queues]);
>> +               xe_wait_ufence(fd, &spin[num_queues]->exec_sync,
>> USER_FENCE_VALUE,
>> +                              exec_queues[num_queues],
>> fence_timeout);
>>          }
>>   
>>          /*
>> @@ -566,6 +585,10 @@ __test_priority(int fd, struct
>> drm_xe_engine_class_instance *eci,
>>          for (i = 0; i < num_queues; i++)
>>                  xe_exec_queue_destroy(fd, exec_queues[i]);
>>   
>> +       /* Destroy the extra queue */
>> +       if (flags & DYN_PRIORITY)
>> +               xe_exec_queue_destroy(fd, exec_queues[num_queues]);
>> +
>>          munmap(bo_map, bo_size);
>>          gem_close(fd, bo);
>>