Attention is currently required from: laforge, neels, pespin.
Hello Jenkins Builder, laforge, neels, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder
Change subject: core: osmo_tdef_fsm_inst_state_chg(): allow millisecond precision
......................................................................
core: osmo_tdef_fsm_inst_state_chg(): allow millisecond precision
This API predates commit 7b74551b9, which added support for millisecond
granularity to osmo_fsm. Let's do the same for the tdef FSM wrapper
API, allowing the millisecond precision without rounding-up to seconds.
Of course, this patch changes behavior of the existing API, but having
more precise state timeouts is not going to make the API user
experience worse.
The old behavior of using seconds is for kept for:
* OSMO_TDEF_CUSTOM -- still treated as if it was OSMO_TDEF_S.
* \param[in] default_timeout -- still expected to be in seconds.
Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1
Related: 7b74551b9 "fsm: Allow millisecond granularity in osmo_fsm built-in timer"
---
M TODO-RELEASE
M include/osmocom/core/tdef.h
M src/core/tdef.c
M tests/tdef/tdef_test.err
M tests/tdef/tdef_test.ok
5 files changed, 57 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/66/35466/6
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1
Gerrit-Change-Number: 35466
Gerrit-PatchSet: 6
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35469?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: tests/tdef: tune logging, also match stderr
......................................................................
tests/tdef: tune logging, also match stderr
Change-Id: I7f346dfbec9e724e905d26990a978495d3a9b030
---
M tests/Makefile.am
M tests/tdef/tdef_test.c
A tests/tdef/tdef_test.err
M tests/testsuite.at
4 files changed, 46 insertions(+), 2 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 478dd48..8d510d5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -466,6 +466,7 @@
oap/oap_client_test.ok oap/oap_client_test.err \
vty/vty_transcript_test.vty \
tdef/tdef_test.ok \
+ tdef/tdef_test.err \
tdef/tdef_test_range_64bit.ok \
tdef/tdef_vty_config_root_test.vty \
tdef/tdef_vty_config_subnode_test.vty \
@@ -676,7 +677,8 @@
gsm23236/gsm23236_test \
>$(srcdir)/gsm23236/gsm23236_test.ok
tdef/tdef_test \
- >$(srcdir)/tdef/tdef_test.ok
+ >$(srcdir)/tdef/tdef_test.ok \
+ 2>$(srcdir)/tdef/tdef_test.err
sockaddr_str/sockaddr_str_test \
>$(srcdir)/sockaddr_str/sockaddr_str_test.ok
use_count/use_count_test \
diff --git a/tests/tdef/tdef_test.c b/tests/tdef/tdef_test.c
index 13dcd01..1cc9c9c 100644
--- a/tests/tdef/tdef_test.c
+++ b/tests/tdef/tdef_test.c
@@ -469,9 +469,16 @@
osmo_init_logging2(ctx, NULL);
log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_NONE);
+ log_set_print_level(osmo_stderr_target, 1);
log_set_print_category(osmo_stderr_target, 1);
+ log_set_print_category_hex(osmo_stderr_target, 0);
log_set_use_color(osmo_stderr_target, 0);
+ osmo_fsm_log_addr(false);
+ osmo_fsm_log_timeouts(true);
+
+ log_set_category_filter(osmo_stderr_target, DLGLOBAL, 1, LOGL_DEBUG);
+
OSMO_ASSERT(osmo_fsm_register(&test_tdef_fsm) == 0);
test_tdef_get(argc > 1);
diff --git a/tests/tdef/tdef_test.err b/tests/tdef/tdef_test.err
new file mode 100644
index 0000000..1a39b89
--- /dev/null
+++ b/tests/tdef/tdef_test.err
@@ -0,0 +1,25 @@
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: Allocated
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to A (T1, 100s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to B (T2, 1s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){B}: State change to C (T3, 3000s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){C}: State change to D (T4, 100s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){D}: State change to E (X5, 1s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){E}: State change to F (X6, 1s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){F}: State change to G (T7, 50s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){G}: State change to H (T8, 300s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){H}: State change to I (T9, 300s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){I}: State change to J (T10, 1200s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){J}: State change to K (keeping T10, 1076.954s remaining)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){K}: State change to A (T1, 100s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to K (keeping T1, 76.954s remaining)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){K}: State change to A (T1, 100s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){A}: State change to L (keeping T1, 76.954s remaining)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){L}: State change to O (no timeout)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){O}: State change to L (T123, 1s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){L}: State change to O (no timeout)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){O}: State change to X (no timeout)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){X}: State change to Y (T666, 999s)
+DLGLOBAL DEBUG tdef_test(test_tdef_state_timeout){Y}: State change to Z (no timeout)
+DLGLOBAL ERROR tdef_test(test_tdef_state_timeout){Z}: transition to state B not permitted!
+DLGLOBAL ERROR tdef_test(test_tdef_state_timeout){Z}: transition to state C not permitted!
+DLGLOBAL ERROR tdef_test(test_tdef_state_timeout){Z}: transition to state D not permitted!
diff --git a/tests/testsuite.at b/tests/testsuite.at
index e88e0a8..40424e1 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -414,7 +414,8 @@
AT_SETUP([tdef])
AT_KEYWORDS([tdef])
cat $abs_srcdir/tdef/tdef_test.ok > expout
-AT_CHECK([$abs_top_builddir/tests/tdef/tdef_test], [0], [expout], [ignore])
+cat $abs_srcdir/tdef/tdef_test.err > experr
+AT_CHECK([$abs_top_builddir/tests/tdef/tdef_test], [0], [expout], [experr])
AT_CLEANUP
AT_SETUP([sockaddr_str])
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35469?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7f346dfbec9e724e905d26990a978495d3a9b030
Gerrit-Change-Number: 35469
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: fixeria.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35473?usp=email )
Change subject: vty: suppress warnings about len being set but not used
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35473?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0040ef20ba3fc53ee7ccefc4885170f333f80566
Gerrit-Change-Number: 35473
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 03 Jan 2024 21:22:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35478?usp=email )
Change subject: regen-makefile.sh: Increase file Code splitting to decrease mem use
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
I confirm I no longer hit OOM situation in my laptop today after the whole day building the testsuite (and kernel cache/used memory growing over time). The memory used by the system while compiling (excluding cache/buffers) peaks around 8-9GB now, which is acceptable in a 16GB host running plenty of other stuff concurrently. The 8 concurrent compiler processes take in total around 4-5GB at peak.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/35478?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If5f54e18384f8a26b281d057e9d9f5c450566422
Gerrit-Change-Number: 35478
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 03 Jan 2024 19:10:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: neels, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35467?usp=email )
Change subject: pseudotalloc: add talloc_memdup(), use it in talloc_strdup()
......................................................................
Patch Set 4:
(1 comment)
File src/pseudotalloc/pseudotalloc.c:
https://gerrit.osmocom.org/c/libosmocore/+/35467/comment/7843356f_230b4745
PS4, Line 61:
> (should it check for size == 0? ... […]
I don't think we should check for every possible mistake the API user can make.
This 'pseudo' implementation is not aiming to be 100% complete either...
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35467?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ifcf377c3496a9e75404932a1aaba7d74888cf4cf
Gerrit-Change-Number: 35467
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 03 Jan 2024 17:18:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email )
Change subject: core: osmo_tdef_fsm_inst_state_chg(): allow millisecond precision
......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS4:
> should we immediately improve to full microsecond granularity, as in not repeat the earlier mistake?
As long as there is a need for such a high granularity...
AFAICS, `OSMO_TDEF_US` timers are not that common in the Osmocom codebase.
@laforge@osmocom.org what do you think?
File src/core/tdef.c:
https://gerrit.osmocom.org/c/libosmocore/+/35466/comment/d2890684_925cc8c5
PS4, Line 347: val = osmo_tdef_get(tdefs, t->T, OSMO_TDEF_S, default_timeout);
> IIRC osmo_tdef_get(..., OSMO_TDEF_MS, ...) should do exactly what is intended here. […]
No, I actually **had to** re-implement the logic of `osmo_tdef_get()` here. The main reason is that we need to check if `tdef->unit == OSMO_TDEF_CUSTOM` and convert the value in seconds to milliseconds. Otherwise, we would suddenly start treating `OSMO_TDEF_CUSTOM` values as milliseconds...
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35466?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1
Gerrit-Change-Number: 35466
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 03 Jan 2024 17:10:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment