diff mbox series

gdb/testsuite/lib/rocm: Fix with_rocm_gpu_lock

Message ID 20240507091723.107082-1-lancelot.six@amd.com
State New
Headers
Series gdb/testsuite/lib/rocm: Fix with_rocm_gpu_lock |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_gdb_build--master-arm warning Patch is already merged

Commit Message

Lancelot SIX May 7, 2024, 9:17 a.m. UTC 
  A recent commit refactored with_rocm_gpu_lock:

    commit fbb0edfe60edf4ca01884151e6d9b1353aaa0a7e
    Date:   Sat May 4 10:41:09 2024 +0200

        [gdb/testsuite] Factor out proc with_lock

        Factor out proc with_lock from with_rocm_gpu_lock, and move required procs
        lock_file_acquire and lock_file_release to lib/gdb-utils.exp.

This causes regressions in gdb.rocm/*.exp (as well as in downstream
ROCgdb).  The issue can be reproduced in the following minimal test:

    load_lib rocm.exp
    set foo hello
    with_rocm_gpu_lock {
        verbose -logs $foo
    }

The issue is that the body to execute under the lock is executed in the
context of with_rocm_gpu_lock (uplevel 1 used in with_lock) instead of
in the context of the "original" caller.

This patch adjusted with_rocm_gpu_lock to account for the new extra
frame in the call stack between the caller of with_rocm_gpu_lock and
where the code execution is triggered.

Change-Id: I79ce2c9615012215867ed5bb60144abe7dce28fe
---
 gdb/testsuite/lib/rocm.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: cdf5362f562ad605199f28db686f93251979ae5d

Comments

Tom de Vries May 7, 2024, 9:33 a.m. UTC | #1 
On 5/7/24 11:17, Lancelot SIX wrote:
> A recent commit refactored with_rocm_gpu_lock:
> 
>      commit fbb0edfe60edf4ca01884151e6d9b1353aaa0a7e
>      Date:   Sat May 4 10:41:09 2024 +0200
> 
>          [gdb/testsuite] Factor out proc with_lock
> 
>          Factor out proc with_lock from with_rocm_gpu_lock, and move required procs
>          lock_file_acquire and lock_file_release to lib/gdb-utils.exp.
> 
> This causes regressions in gdb.rocm/*.exp (as well as in downstream
> ROCgdb).  The issue can be reproduced in the following minimal test:
> 
>      load_lib rocm.exp
>      set foo hello
>      with_rocm_gpu_lock {
>          verbose -logs $foo
>      }
> 
> The issue is that the body to execute under the lock is executed in the
> context of with_rocm_gpu_lock (uplevel 1 used in with_lock) instead of
> in the context of the "original" caller.
> 
> This patch adjusted with_rocm_gpu_lock to account for the new extra
> frame in the call stack between the caller of with_rocm_gpu_lock and
> where the code execution is triggered.
> 

Hi Lancelot,

sorry for the breakage, thanks for fixing this.

LGTM.

Approved-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> Change-Id: I79ce2c9615012215867ed5bb60144abe7dce28fe
> ---
>   gdb/testsuite/lib/rocm.exp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
> index 2276bb3640e..b2db0d5c783 100644
> --- a/gdb/testsuite/lib/rocm.exp
> +++ b/gdb/testsuite/lib/rocm.exp
> @@ -112,7 +112,7 @@ set gpu_lock_filename gpu-parallel.lock
>   # the GPU lock.
>   
>   proc with_rocm_gpu_lock { body } {
> -    with_lock $::gpu_lock_filename $body
> +    with_lock $::gpu_lock_filename {uplevel 1 $body}
>   
>       # In case BODY returned early due to some testcase failing, and
>       # left GDB running, debugging the GPU.
> 
> base-commit: cdf5362f562ad605199f28db686f93251979ae5d
Lancelot SIX May 7, 2024, 9:37 a.m. UTC | #2 
> sorry for the breakage, thanks for fixing this.

No problem, that was an easy fix.

> 
> LGTM.
> 
> Approved-By: Tom de Vries <tdevries@suse.de>

Thanks,
I have pushed this patch.

Best,
Lancelot.
diff mbox series

Patch

diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index 2276bb3640e..b2db0d5c783 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -112,7 +112,7 @@  set gpu_lock_filename gpu-parallel.lock
 # the GPU lock.
 
 proc with_rocm_gpu_lock { body } {
-    with_lock $::gpu_lock_filename $body
+    with_lock $::gpu_lock_filename {uplevel 1 $body}
 
     # In case BODY returned early due to some testcase failing, and
     # left GDB running, debugging the GPU.