<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> The change looks good to me. Please do some tiny modifications below. It is related to explicitly having a code block for timeout avoid checking: if (sh_mem->reset_flags == 0) This prevents a potential race condition between processes. Since a timeout implies that we do not need to check the flags, this check can be safely omitted. Reviewed-by: Vitaly Prosyak <a class="moz-txt-link-abbreviated" href="mailto:vitaly.prosyak@amd.com">vitaly.prosyak@amd.com</a> <div class="moz-cite-prefix">On 2024-08-14 06:11, <a class="moz-txt-link-abbreviated" href="mailto:Jesse.zhang@amd.com">Jesse.zhang@amd.com</a> wrote: </div> <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap="">1.If the test case cannot trigger any reset on some ASIC, It should be considered a failure. 2. Fix code style Signed-off-by: Jesse Zhang <a class="moz-txt-link-rfc2396E" href="mailto:jesse.zhang@amd.com"><jesse.zhang@amd.com></a> --- tests/amdgpu/amd_queue_reset.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/amdgpu/amd_queue_reset.c b/tests/amdgpu/amd_queue_reset.c index 6819892e0..249676407 100644 --- a/tests/amdgpu/amd_queue_reset.c +++ b/tests/amdgpu/amd_queue_reset.c @@ -30,6 +30,7 @@ #define SHARED_CHILD_DESCRIPTOR 3 #define SHARED_MEM_NAME "/queue_reset_shm" +#define TEST_TIMEOUT 100 //100 seconds enum process_type { PROCESS_UNKNOWN, @@ -49,6 +50,7 @@ enum error_code_bits { }; enum reset_code_bits { + NO_RESET_SET_BIT, QUEUE_RESET_SET_BIT, GPU_RESET_BEGIN_SET_BIT, GPU_RESET_END_SUCCESS_SET_BIT,</pre> </blockquote> Change to ALL_RESET_BITS = 0x1f, Since we have now 5 states after you added NO_RESET_SET_BIT <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap=""> @@ -307,6 +309,7 @@ static void set_next_test_to_run(struct shmbuf *sh_mem, unsigned int error, sync_point_enter(sh_mem); wait_for_complete_iteration(sh_mem); sync_point_exit(sh_mem); + igt_assert_neq(sh_mem->reset_flags, 1U << NO_RESET_SET_BIT); } static int @@ -473,6 +476,9 @@ run_monitor_child(amdgpu_device_handle device, amdgpu_context_handle *arr_contex int state_machine = 0; int error_code; unsigned int flags; + int64_t cnt = 0; + time_t start, end; + double elapsed = 0; after_reset_state = after_reset_hangs = 0; init_flags = in_process_flags = 0; @@ -487,7 +493,8 @@ run_monitor_child(amdgpu_device_handle device, amdgpu_context_handle *arr_contex error_code = 0; flags = 0; set_reset_state(sh_mem, false, ALL_RESET_BITS); - while (1) {</pre> </blockquote> Please, keep while(1) <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap=""> + time(&start); + while (elapsed < TEST_TIMEOUT) { if (state_machine == 0) { amdgpu_cs_query_reset_state2(arr_context[test_counter], &init_flags); @@ -533,7 +540,15 @@ run_monitor_child(amdgpu_device_handle device, amdgpu_context_handle *arr_contex break; } } + cnt++; + if (cnt % 1000000 == 0) { + time(&end); + elapsed = difftime(end, start);</pre> </blockquote> if ( elapsed >= TEST_TIMEOUT) { <pre class="moz-quote-pre" wrap=""> set_reset_state(sh_mem, true, NO_RESET_SET_BIT); break; </pre> } <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap="">+ } } + elapsed = 0;</pre> </blockquote> remove 2 lines below: <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap=""> + if (sh_mem->reset_flags == 0) //remove this + set_reset_state(sh_mem, true, NO_RESET_SET_BIT);</pre> </blockquote> <blockquote type="cite" cite="mid:20240814101140.3165345-1-jesse.zhang@amd.com"> <pre class="moz-quote-pre" wrap=""> sync_point_exit(sh_mem); num_of_tests--; test_counter++; @@ -1000,7 +1015,7 @@ igt_main igt_describe("Stressful-and-multiple-cs-of-bad and good length-operations-using-multiple-processes"); igt_subtest_with_dynamic_f("amdgpu-%s-%s", ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":"GRAFIX", it->name) { if (arr_cap[ip_tests[i]] && get_next_rings(ring_id_good, info, &ring_id_good, &ring_id_bad, i)) { - igt_dynamic_f("amdgpu-%s-ring-good-%d-bad-%d-%s", it->name,ring_id_good, ring_id_bad, ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":"GRAFIX") + igt_dynamic_f("amdgpu-%s-ring-good-%d-bad-%d-%s", it->name, ring_id_good, ring_id_bad, ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":"GRAFIX") set_next_test_to_run(sh_mem, it->test, ip_background, ip_tests[i], ring_id_good, ring_id_bad); } } </pre> </blockquote> </body> </html>