* [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum
@ 2024-04-15 15:56 Tom de Vries
2024-04-15 15:56 ` [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global Tom de Vries
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
In gdbserver_start, we have some code that determines what port number to use:
...
# Port id -- either specified in baseboard file, or managed here.
if [target_info exists gdb,socketport] {
set portnum [target_info gdb,socketport]
} else {
# Bump the port number to avoid conflicts with hung ports.
incr portnum
}
...
Factor this out into a new proc get_portnum.
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdbserver-support.exp | 40 ++++++++++++++++++-------
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 727a66e2ab1..bf000119db6 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -129,8 +129,31 @@ proc gdb_target_cmd { args } {
return [expr $res == 0 ? 0 : 1]
}
-global portnum
-set portnum "2345"
+# Return a usable port number.
+
+proc get_portnum {} {
+ if { [target_info exists gdb,socketport] } {
+ # Hard-coded in target board.
+ return [target_info gdb,socketport]
+ }
+
+ # Not hard-coded in target board. Return increasing port numbers,
+ # starting at $initial_portnum, to avoid conflicts with hung ports.
+ set initial_portnum 2345
+
+ # Currently available port number.
+ global portnum
+
+ # Initialize, if necessary.
+ if { ![info exists portnum] } {
+ set portnum $initial_portnum
+ }
+
+ # Return currently available port number, and update it.
+ set res $portnum
+ incr portnum
+ return $res
+}
# Locate the gdbserver binary. Returns "" if gdbserver could not be found.
@@ -247,16 +270,10 @@ proc gdbserver_default_get_comm_port { port } {
# Returns the target protocol and socket to connect to.
proc gdbserver_start { options arguments } {
- global portnum
global GDB_TEST_SOCKETHOST
# Port id -- either specified in baseboard file, or managed here.
- if [target_info exists gdb,socketport] {
- set portnum [target_info gdb,socketport]
- } else {
- # Bump the port number to avoid conflicts with hung ports.
- incr portnum
- }
+ set portnum [get_portnum]
# Extract the local and remote host ids from the target board struct.
if { [info exists GDB_TEST_SOCKETHOST] } {
@@ -372,10 +389,11 @@ proc gdbserver_start { options arguments } {
-re "Listening on" { }
-re "Can't (bind address|listen on socket): Address already in use\\.\r\n" {
verbose -log "Port $portnum is already in use."
- if ![target_info exists gdb,socketport] {
+ set other_portnum [get_portnum]
+ if { $other_portnum != $portnum } {
# Bump the port number to avoid the conflict.
wait -i $expect_out(spawn_id)
- incr portnum
+ set portnum $other_portnum
continue
}
}
base-commit: 9dd918142787246ea7ed53494d9cbc6b51486133
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:20 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock Tom de Vries
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
When instrumenting get_portnum using:
...
puts "PORTNUM: $res"
...
and running:
...
$ cd build/gdb
$ make check TESTS=gdb.server/*.exp
...
we get:
...
Running gdb.server/target-exec-file.exp ...
PORTNUM: 2345
Running gdb.server/stop-reply-no-thread-multi.exp ...
PORTNUM: 2345
PORTNUM: 2346
PORTNUM: 2347
PORTNUM: 2348
PORTNUM: 2349
PORTNUM: 2350
...
So, while get_portnum does return increasing numbers in a single test-case, it
restarts at each test-case.
This is a regression since the introduction of persistent globals.
Fix this by using "gdb_persistent_global portnum", such that we get:
...
Running gdb.server/target-exec-file.exp ...
PORTNUM: 2345
Running gdb.server/stop-reply-no-thread-multi.exp ...
PORTNUM: 2346
PORTNUM: 2347
PORTNUM: 2348
PORTNUM: 2349
PORTNUM: 2350
PORTNUM: 2351
...
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdbserver-support.exp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index bf000119db6..0f97ce9c0fd 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -142,7 +142,7 @@ proc get_portnum {} {
set initial_portnum 2345
# Currently available port number.
- global portnum
+ gdb_persistent_global portnum
# Initialize, if necessary.
if { ![info exists portnum] } {
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
2024-04-15 15:56 ` [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:22 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir Tom de Vries
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
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.
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdb-utils.exp | 59 +++++++++++++++++++++++++++++++++
gdb/testsuite/lib/rocm.exp | 55 +-----------------------------
2 files changed, 60 insertions(+), 54 deletions(-)
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index 34752081b60..4205f8d1a22 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -138,3 +138,62 @@ proc version_compare { l1 op l2 } {
}
return 1
}
+
+# Acquire lock file LOCKFILE. Tries forever until the lock file is
+# successfully created.
+
+proc lock_file_acquire {lockfile} {
+ verbose -log "acquiring lock file: $::subdir/${::gdb_test_file_name}.exp"
+ while {true} {
+ if {![catch {open $lockfile {WRONLY CREAT EXCL}} rc]} {
+ set msg "locked by $::subdir/${::gdb_test_file_name}.exp"
+ verbose -log "lock file: $msg"
+ # For debugging, put info in the lockfile about who owns
+ # it.
+ puts $rc $msg
+ flush $rc
+ return [list $rc $lockfile]
+ }
+ after 10
+ }
+}
+
+# Release a lock file.
+
+proc lock_file_release {info} {
+ verbose -log "releasing lock file: $::subdir/${::gdb_test_file_name}.exp"
+
+ if {![catch {fconfigure [lindex $info 0]}]} {
+ if {![catch {
+ close [lindex $info 0]
+ file delete -force [lindex $info 1]
+ } rc]} {
+ return ""
+ } else {
+ return -code error "Error releasing lockfile: '$rc'"
+ }
+ } else {
+ error "invalid lock"
+ }
+}
+
+# Run body under lock LOCK_FILE.
+
+proc with_lock { lock_file body } {
+ if {[info exists ::GDB_PARALLEL]} {
+ set lock_rc [lock_file_acquire $lock_file]
+ }
+
+ set code [catch {uplevel 1 $body} result]
+
+ if {[info exists ::GDB_PARALLEL]} {
+ lock_file_release $lock_rc
+ }
+
+ if {$code == 1} {
+ global errorInfo errorCode
+ return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+ } else {
+ return -code $code $result
+ }
+}
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index ab21db6685c..7dd7ef3f3b5 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -108,68 +108,15 @@ gdb_caching_proc allow_hipcc_tests {} {
# at a time.
set gpu_lock_filename $objdir/gpu-parallel.lock
-# Acquire lock file LOCKFILE. Tries forever until the lock file is
-# successfully created.
-
-proc lock_file_acquire {lockfile} {
- verbose -log "acquiring lock file: $::subdir/${::gdb_test_file_name}.exp"
- while {true} {
- if {![catch {open $lockfile {WRONLY CREAT EXCL}} rc]} {
- set msg "locked by $::subdir/${::gdb_test_file_name}.exp"
- verbose -log "lock file: $msg"
- # For debugging, put info in the lockfile about who owns
- # it.
- puts $rc $msg
- flush $rc
- return [list $rc $lockfile]
- }
- after 10
- }
-}
-
-# Release a lock file.
-
-proc lock_file_release {info} {
- verbose -log "releasing lock file: $::subdir/${::gdb_test_file_name}.exp"
-
- if {![catch {fconfigure [lindex $info 0]}]} {
- if {![catch {
- close [lindex $info 0]
- file delete -force [lindex $info 1]
- } rc]} {
- return ""
- } else {
- return -code error "Error releasing lockfile: '$rc'"
- }
- } else {
- error "invalid lock"
- }
-}
-
# Run body under the GPU lock. Also calls gdb_exit before releasing
# the GPU lock.
proc with_rocm_gpu_lock { body } {
- if {[info exists ::GDB_PARALLEL]} {
- set lock_rc [lock_file_acquire $::gpu_lock_filename]
- }
-
- set code [catch {uplevel 1 $body} result]
+ with_lock $::gpu_lock_filename $body
# In case BODY returned early due to some testcase failing, and
# left GDB running, debugging the GPU.
gdb_exit
-
- if {[info exists ::GDB_PARALLEL]} {
- lock_file_release $lock_rc
- }
-
- if {$code == 1} {
- global errorInfo errorCode
- return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
- } else {
- return -code $code $result
- }
}
# Return true if all the devices support debugging multiple processes
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
2024-04-15 15:56 ` [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global Tom de Vries
2024-04-15 15:56 ` [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:23 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir Tom de Vries
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
In lib/rocm.exp we have:
...
set gpu_lock_filename $objdir/gpu-parallel.lock
...
This decides both the lock file name and directory.
Factor out a new proc lock_dir that decides on the directory, leaving just:
...
set gpu_lock_filename gpu-parallel.lock
...
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdb-utils.exp | 7 +++++++
gdb/testsuite/lib/rocm.exp | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index 4205f8d1a22..c0b96d3c2bf 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -177,10 +177,17 @@ proc lock_file_release {info} {
}
}
+# Return directory where we keep lock files.
+
+proc lock_dir {} {
+ return $objdir
+}
+
# Run body under lock LOCK_FILE.
proc with_lock { lock_file body } {
if {[info exists ::GDB_PARALLEL]} {
+ set lock_file [file join [lock_dir] $lock_file]
set lock_rc [lock_file_acquire $lock_file]
}
diff --git a/gdb/testsuite/lib/rocm.exp b/gdb/testsuite/lib/rocm.exp
index 7dd7ef3f3b5..2276bb3640e 100644
--- a/gdb/testsuite/lib/rocm.exp
+++ b/gdb/testsuite/lib/rocm.exp
@@ -106,7 +106,7 @@ gdb_caching_proc allow_hipcc_tests {} {
# The lock file used to ensure that only one GDB has access to the GPU
# at a time.
-set gpu_lock_filename $objdir/gpu-parallel.lock
+set gpu_lock_filename gpu-parallel.lock
# Run body under the GPU lock. Also calls gdb_exit before releasing
# the GPU lock.
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
` (2 preceding siblings ...)
2024-04-15 15:56 ` [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:25 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing Tom de Vries
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
The lock directory returned by lock_dir is currently $objdir.
It seems possible to leave a stale lock file that blocks progress in a
following run.
Fix this by using a directory that is guaranteed to be initially empty when
using GDB_PARALLEL, like temp or cache.
In gdb/testsuite/README I found:
...
cache in particular is used to share data across invocations of runtest
...
which seems appropriate, so let's use cache for this.
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdb-utils.exp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index c0b96d3c2bf..1f30d807ff0 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -180,7 +180,7 @@ proc lock_file_release {info} {
# Return directory where we keep lock files.
proc lock_dir {} {
- return $objdir
+ return [make_gdb_parallel_path cache]
}
# Run body under lock LOCK_FILE.
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
` (3 preceding siblings ...)
2024-04-15 15:56 ` [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:27 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case) Tom de Vries
2024-05-03 20:19 ` [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom Tromey
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
When instrumenting get_portnum using:
...
puts "PORTNUM: $res"
...
and running:
...
$ cd build/gdb
$ make check-parallel -j2 TESTS=gdb.server/*.exp
...
we run into:
...
Running gdb.server/abspath.exp ...
PORTNUM: 2345
...
and:
...
Running gdb.server/bkpt-other-inferior.exp ...
PORTNUM: 2345
...
This is because the test-cases are run in independent runtest invocations.
Fix this by handling the parallel case in get_portnum using:
- a file $objdir/cache/portnum to keep the portnum variable, and
- a file $objdir/cache/portnum.lock to serialize access to it.
Tested on aarch64-linux.
---
gdb/testsuite/lib/gdbserver-support.exp | 47 ++++++++++++++++++++-----
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 0f97ce9c0fd..41ad5e6cbfb 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -141,18 +141,47 @@ proc get_portnum {} {
# starting at $initial_portnum, to avoid conflicts with hung ports.
set initial_portnum 2345
- # Currently available port number.
- gdb_persistent_global portnum
+ if { ![info exists ::GDB_PARALLEL] } {
+ # Sequential case.
- # Initialize, if necessary.
- if { ![info exists portnum] } {
- set portnum $initial_portnum
+ # Currently available port number.
+ gdb_persistent_global portnum
+
+ # Initialize, if necessary.
+ if { ![info exists portnum] } {
+ set portnum $initial_portnum
+ }
+
+ # Return currently available port number, and update it.
+ set res $portnum
+ incr portnum
+ return $res
+ }
+
+ # Parallel case.
+ with_lock portnum.lock {
+ # Keep portnum file alongside the lock that guards it.
+ set portnum_file [lock_dir]/portnum
+
+ if { [file exists $portnum_file] } {
+ set fd [open $portnum_file r]
+ set portnum [read $fd]
+ close $fd
+
+ set portnum [string trim $portnum]
+ } else {
+ # Initialize.
+ set portnum $initial_portnum
+ }
+
+ set next_portnum [expr $portnum + 1]
+
+ set fd [open $portnum_file w]
+ puts $fd $next_portnum
+ close $fd
}
- # Return currently available port number, and update it.
- set res $portnum
- incr portnum
- return $res
+ return $portnum
}
# Locate the gdbserver binary. Returns "" if gdbserver could not be found.
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case)
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
` (4 preceding siblings ...)
2024-04-15 15:56 ` [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing Tom de Vries
@ 2024-04-15 15:56 ` Tom de Vries
2024-05-03 20:30 ` Tom Tromey
2024-05-03 20:19 ` [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom Tromey
6 siblings, 1 reply; 15+ messages in thread
From: Tom de Vries @ 2024-04-15 15:56 UTC (permalink / raw)
To: gdb-patches
Make target check//% is the gdb variant of a similar gcc make target [1].
When running tests using check//%:
...
$ cd build/gdb
$ make check//unix/{-fPIE/-pie,-fno-PIE/-no-pie} -j2 TESTS=gdb.server/*.exp
...
we get:
...
$ cat build/gdb/testsuite.unix.-fPIE.-pie/cache/portnum
2427
$ cat build/gdb/testsuite.unix.-fno-PIE.-no-pie/cache/portnum
2423
...
The problem is that there are two portnum files used in parallel.
Fix this by:
- creating a common lockdir build/gdb/testsuite.lockdir for make target
check//%,
- passing this down to the runtests invocations using variable GDB_LOCK_DIR,
and
- using GDB_LOCK_DIR in lock_dir.
Tested on aarch64-linux.
PR testsuite/31632
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31632
[1] https://gcc.gnu.org/install/test.html
---
gdb/Makefile.in | 8 ++++++--
gdb/testsuite/lib/gdb-utils.exp | 5 +++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 23894ea4a4d..5a63f166107 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2002,6 +2002,10 @@ check-all-boards: force
$(MAKE) $(TARGET_FLAGS_TO_PASS) check-all-boards; \
else true; fi
+testsuite.lockdir: force
+ rm -rf $@
+ mkdir -p $@
+
# The idea is to parallelize testing of multilibs, for example:
# make -j3 check//sh-hms-sim/{-m1,-m2,-m3,-m3e,-m4}/{,-nofpu}
# will run 3 concurrent sessions of check, eventually testing all 10
@@ -2010,7 +2014,7 @@ check-all-boards: force
# used, this rule will harmlessly fail to match. Used FORCE_PARALLEL to
# prevent serialized checking due to the passed RUNTESTFLAGS.
# FIXME: use config.status --config not --version, when available.
-check//%: force
+check//%: force testsuite.lockdir
@if [ -f testsuite/config.status ]; then \
rootme=`pwd`; export rootme; \
rootsrc=`cd $(srcdir); pwd`; export rootsrc; \
@@ -2028,7 +2032,7 @@ check//%: force
); \
else :; fi && cd $$testdir && \
$(MAKE) $(TARGET_FLAGS_TO_PASS) \
- RUNTESTFLAGS="--target_board=$$variant $(RUNTESTFLAGS)" \
+ RUNTESTFLAGS="GDB_LOCK_DIR=$$rootme/testsuite.lockdir --target_board=$$variant $(RUNTESTFLAGS)" \
FORCE_PARALLEL=$(if $(FORCE_PARALLEL),1,$(if $(RUNTESTFLAGS),,1)) \
"$$target"; \
else true; fi
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index 1f30d807ff0..95c53d030d8 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -180,6 +180,11 @@ proc lock_file_release {info} {
# Return directory where we keep lock files.
proc lock_dir {} {
+ if { [info exists ::GDB_LOCK_DIR] } {
+ # When using check//.
+ return $::GDB_LOCK_DIR
+ }
+
return [make_gdb_parallel_path cache]
}
--
2.35.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
` (5 preceding siblings ...)
2024-04-15 15:56 ` [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case) Tom de Vries
@ 2024-05-03 20:19 ` Tom Tromey
6 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:19 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> In gdbserver_start, we have some code that determines what port number to use:
Tom> ...
Tom> # Port id -- either specified in baseboard file, or managed here.
Tom> if [target_info exists gdb,socketport] {
Tom> set portnum [target_info gdb,socketport]
Tom> } else {
Tom> # Bump the port number to avoid conflicts with hung ports.
Tom> incr portnum
Tom> }
Tom> ...
Tom> Factor this out into a new proc get_portnum.
Tom> Tested on aarch64-linux.
Looks good.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global
2024-04-15 15:56 ` [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global Tom de Vries
@ 2024-05-03 20:20 ` Tom Tromey
0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:20 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> This is a regression since the introduction of persistent globals.
Tom> Fix this by using "gdb_persistent_global portnum", such that we get:
Ok.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock
2024-04-15 15:56 ` [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock Tom de Vries
@ 2024-05-03 20:22 ` Tom Tromey
0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:22 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Factor out proc with_lock from with_rocm_gpu_lock, and move required procs
Tom> lock_file_acquire and lock_file_release to lib/gdb-utils.exp.
Looks good.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir
2024-04-15 15:56 ` [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir Tom de Vries
@ 2024-05-03 20:23 ` Tom Tromey
0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:23 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> In lib/rocm.exp we have:
Tom> ...
Tom> set gpu_lock_filename $objdir/gpu-parallel.lock
Tom> ...
Tom> This decides both the lock file name and directory.
Tom> Factor out a new proc lock_dir that decides on the directory, leaving just:
Tom> ...
Tom> set gpu_lock_filename gpu-parallel.lock
Tom> ...
Ok
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir
2024-04-15 15:56 ` [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir Tom de Vries
@ 2024-05-03 20:25 ` Tom Tromey
0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:25 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> The lock directory returned by lock_dir is currently $objdir.
Tom> It seems possible to leave a stale lock file that blocks progress in a
Tom> following run.
Yeah. Normally I think the idea is that 'make check' first deletes
things that might cause problems.
Tom> Fix this by using a directory that is guaranteed to be initially empty when
Tom> using GDB_PARALLEL, like temp or cache.
Makes sense to me.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing
2024-04-15 15:56 ` [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing Tom de Vries
@ 2024-05-03 20:27 ` Tom Tromey
0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:27 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> This is because the test-cases are run in independent runtest invocations.
Tom> Fix this by handling the parallel case in get_portnum using:
Tom> - a file $objdir/cache/portnum to keep the portnum variable, and
Tom> - a file $objdir/cache/portnum.lock to serialize access to it.
Thank you.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case)
2024-04-15 15:56 ` [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case) Tom de Vries
@ 2024-05-03 20:30 ` Tom Tromey
2024-05-04 7:48 ` Tom de Vries
0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2024-05-03 20:30 UTC (permalink / raw)
To: Tom de Vries; +Cc: gdb-patches
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> Fix this by:
Tom> - creating a common lockdir build/gdb/testsuite.lockdir for make target
Tom> check//%,
Tom> - passing this down to the runtests invocations using variable GDB_LOCK_DIR,
Tom> and
Tom> - using GDB_LOCK_DIR in lock_dir.
I think I've never once used check//.
I'm not even sure I was aware of its existence.
Tom> proc lock_dir {} {
Tom> + if { [info exists ::GDB_LOCK_DIR] } {
Tom> + # When using check//.
Tom> + return $::GDB_LOCK_DIR
Tom> + }
Tom> +
Tom> return [make_gdb_parallel_path cache]
Tom> }
I guess this approach of not always requiring GDB_LOCK_DIR is handy when
running runtest by hand... I was thinking it might be better to unify
all the scenarios here, but really this doesn't seem so complicated.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case)
2024-05-03 20:30 ` Tom Tromey
@ 2024-05-04 7:48 ` Tom de Vries
0 siblings, 0 replies; 15+ messages in thread
From: Tom de Vries @ 2024-05-04 7:48 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 5/3/24 22:30, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> Fix this by:
> Tom> - creating a common lockdir build/gdb/testsuite.lockdir for make target
> Tom> check//%,
> Tom> - passing this down to the runtests invocations using variable GDB_LOCK_DIR,
> Tom> and
> Tom> - using GDB_LOCK_DIR in lock_dir.
>
> I think I've never once used check//.
> I'm not even sure I was aware of its existence.
>
I know it from the suse / opensuse gdb package (
https://build.opensuse.org/projects/devel:gcc/packages/gdb/files/gdb.spec?expand=1
), which I think inherited it from the fedora gdb package.
> Tom> proc lock_dir {} {
> Tom> + if { [info exists ::GDB_LOCK_DIR] } {
> Tom> + # When using check//.
> Tom> + return $::GDB_LOCK_DIR
> Tom> + }
> Tom> +
> Tom> return [make_gdb_parallel_path cache]
> Tom> }
>
> I guess this approach of not always requiring GDB_LOCK_DIR is handy when
> running runtest by hand... I was thinking it might be better to unify
> all the scenarios here, but really this doesn't seem so complicated.
>
> Approved-By: Tom Tromey <tom@tromey.com>
>
Thanks for the review, I'll commit after a round of retesting.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-05-04 7:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-15 15:56 [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom de Vries
2024-04-15 15:56 ` [PATCH 2/7] [gdb/testsuite] Make portnum a persistent global Tom de Vries
2024-05-03 20:20 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 3/7] [gdb/testsuite] Factor out proc with_lock Tom de Vries
2024-05-03 20:22 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 4/7] [gdb/testsuite] Factor out proc lock_dir Tom de Vries
2024-05-03 20:23 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 5/7] [gdb/testsuite] Move gpu-parallel.lock to cache dir Tom de Vries
2024-05-03 20:25 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 6/7] [gdb/testsuite] Use unique portnum in parallel testing Tom de Vries
2024-05-03 20:27 ` Tom Tromey
2024-04-15 15:56 ` [PATCH 7/7] [gdb/testsuite] Use unique portnum in parallel testing (check//% case) Tom de Vries
2024-05-03 20:30 ` Tom Tromey
2024-05-04 7:48 ` Tom de Vries
2024-05-03 20:19 ` [PATCH 1/7] [gdb/testsuite] Factor out proc get_portnum Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).