[LTP] [PATCH 1/3] tst_fuzzy_sync: refine code and comment

[LTP] [PATCH 1/3] tst_fuzzy_sync: refine code and comment

[Li Wang]

I just polished some things preventing me from understanding the
test logic. Hopefully, it makes things easier to read.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 lib/newlib_tests/tst_fuzzy_sync01.c | 22 ++++++------
 lib/newlib_tests/tst_fuzzy_sync02.c | 53 ++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 20 deletions(-)

diff --git a/lib/newlib_tests/tst_fuzzy_sync01.c b/lib/newlib_tests/tst_fuzzy_sync01.c
index d0748958c..d510bd2dd 100644
--- a/lib/newlib_tests/tst_fuzzy_sync01.c
+++ b/lib/newlib_tests/tst_fuzzy_sync01.c
@@ -10,7 +10,7 @@
  *
  * We make the simplifying assumptions that:
  * - Each thread contains a single contiguous critical section.
- * - The threads only interact through a single variable.
+ * - The threads only interact through a single variable(H: Hit).
  * - The various timings are constant except for variations introduced
  *   by the environment.
  *
@@ -50,9 +50,9 @@
  * range is required.
  *
  * When entering their critical sections, both threads increment the
- * 'c' counter variable atomically. They both also increment it when
- * leaving their critical sections. We record the value of 'c' when A
- * increments it. From the recorded values of 'c' we can deduce if the
+ * 'H' counter variable atomically. They both also increment it when
+ * leaving their critical sections. We record the value of 'H' when A
+ * increments it. From the recorded values of 'H' we can deduce if the
  * critical sections overlap and their ordering.
  *
  * 	Start (cs)	| End (ct)	| Ordering
@@ -90,7 +90,7 @@ struct race {
 	const struct window b;
 };
 
-static int c;
+static int H;
 static struct tst_fzsync_pair pair;
 
 static const struct race races[] = {
@@ -162,15 +162,15 @@ static void *worker(void *v)
 	const struct window b = races[i].b;
 
 	while (tst_fzsync_run_b(&pair)) {
-		if (tst_atomic_load(&c))
+		if (tst_atomic_load(&H))
 			tst_brk(TBROK, "Counter should now be zero");
 
 		tst_fzsync_start_race_b(&pair);
 		delay(b.critical_s);
 
-		tst_atomic_add_return(1, &c);
+		tst_atomic_add_return(1, &H);
 		delay(b.critical_t);
-		tst_atomic_add_return(1, &c);
+		tst_atomic_add_return(1, &H);
 
 		delay(b.return_t);
 		tst_fzsync_end_race_b(&pair);
@@ -192,9 +192,9 @@ static void run(unsigned int i)
 		tst_fzsync_start_race_a(&pair);
 		delay(a.critical_s);
 
-		cs = tst_atomic_add_return(1, &c);
+		cs = tst_atomic_add_return(1, &H);
 		delay(a.critical_t);
-		ct = tst_atomic_add_return(1, &c);
+		ct = tst_atomic_add_return(1, &H);
 
 		delay(a.return_t);
 		tst_fzsync_end_race_a(&pair);
@@ -206,7 +206,7 @@ static void run(unsigned int i)
 		else
 			critical++;
 
-		r = tst_atomic_add_return(-4, &c);
+		r = tst_atomic_add_return(-4, &H);
 		if (r)
 			tst_brk(TBROK, "cs = %d, ct = %d, r = %d", cs, ct, r);
 
diff --git a/lib/newlib_tests/tst_fuzzy_sync02.c b/lib/newlib_tests/tst_fuzzy_sync02.c
index afe4973b5..f95394371 100644
--- a/lib/newlib_tests/tst_fuzzy_sync02.c
+++ b/lib/newlib_tests/tst_fuzzy_sync02.c
@@ -11,7 +11,7 @@
  * We make the simplifying assumptions that:
  * - There is one data race we want to hit and one to avoid.
  * - Each thread contains two contiguous critical sections. One for each race.
- * - The threads only interact through two variables, one for each race.
+ * - The threads only interact through two variables(H: Hit, D: Avoid), one for each race.
  * - If we hit the race we want to avoid then it causes thread A to exit early.
  *
  * We don't consider more complicated dynamic interactions between the
@@ -63,14 +63,35 @@ struct race {
 	const struct window b;
 };
 
-static int c, d;
+static int H, D;
 static struct tst_fzsync_pair pair;
 
+/**
+ * Race 1:
+ *  Thread A:   |---(1)--|[1]|---(1)---|
+ *  Thread B:   |---(1)--|[1]|---(1)---|
+ *  ad (A):     |---(0)|[1]|(0)---|
+ *  bd (B):     |---(0)|[1]|(0)---|
+ *
+ * Race 2:
+ *  Thread A:   |------------------(30)------------------|[1]|---(1)---|
+ *  Thread B:   |---(1)--|[1]|---(1)---|
+ *  ad (A):     |---(0)|[1]|--(0)---|
+ *  bd (B):     |---(0)|[20]|--(0)---|
+ *
+ * Race 3:
+ *  Thread A:   |--------------------(40)--------------------|[1]|---(0)---|
+ *  Thread B:   |---(1)--|[1]|------------------(20)------------------|
+ *  ad (A):     |---(1)--|[10]|--(0)---|
+ *  bd (B):     |---(1)--|[10]|--(0)---|
+ */
 static const struct race races[] = {
 	{ .a =  { 1, 1, 1 }, .b =  { 1, 1, 1 },
 	  .ad = { 0, 1, 0 }, .bd = { 0, 1, 0 } },
+
 	{ .a =  { 30, 1, 1 }, .b =  { 1, 1,  1 },
 	  .ad = { 0,  1, 0 }, .bd = { 0, 20, 0 } },
+
 	{ .a =  { 40, 1,  0 }, .b =  { 1, 1,  20 },
 	  .ad = { 1,  10, 0 }, .bd = { 1, 10, 0 } },
 };
@@ -87,6 +108,20 @@ static void setup(void)
 	tst_fzsync_pair_init(&pair);
 }
 
+/**
+ * to_abs() - Convert relative time intervals to absolute time points
+ * @w: The input window structure containing relative time intervals
+ *
+ * This function converts relative time intervals (represented in the
+ * struct window) into absolute time points, where:
+ *
+ * - The start of the critical section is `critical_s`.
+ * - The end of the critical section is calculated as `critical_s + critical_t`.
+ * - The end of execution is calculated as `critical_s + critical_t + return_t`.
+ *
+ * Return:
+ * A new window structure (`wc`) with absolute time points.
+ */
 static struct window to_abs(const struct window w)
 {
 	const struct window wc = {
@@ -109,9 +144,9 @@ static void *worker(void *v)
 		tst_fzsync_start_race_b(&pair);
 		for (now = 0; now <= fin; now++) {
 			if (now == b.critical_s || now == b.critical_t)
-				tst_atomic_add_return(1, &c);
+				tst_atomic_add_return(1, &H);
 			if (now == bd.critical_s || now == bd.critical_t)
-				tst_atomic_add_return(1, &d);
+				tst_atomic_add_return(1, &D);
 
 			sched_yield();
 		}
@@ -132,19 +167,19 @@ static void run(unsigned int i)
 	SAFE_PTHREAD_CREATE(&pair.thread_b, 0, worker, &i);
 
 	while (tst_fzsync_run_a(&pair)) {
-		c = 0;
-		d = 0;
+		H = 0;
+		D = 0;
 		fin = a.return_t;
 
 		tst_fzsync_start_race_a(&pair);
 		for (now = 0; now <= fin; now++) {
 			if (now >= ad.critical_s &&
-			    now <= ad.critical_t && tst_atomic_load(&d) > 0)
+			    now <= ad.critical_t && tst_atomic_load(&D) > 0)
 				fin = ad.return_t;
 
 			if (now >= a.critical_s &&
-			    now <= a.critical_t && tst_atomic_load(&c) == 1) {
-				tst_atomic_add_return(1, &c);
+			    now <= a.critical_t && tst_atomic_load(&H) == 1) {
+				tst_atomic_add_return(1, &H);
 				critical++;
 			}
 
-- 
2.47.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] tst_fuzzy_sync: ingnore test result if loop termination occurs

[Li Wang]

This patch updates the behavior of tst_fuzzy_sync tests to address
scenarios where test results may be unreliable due to resource constraints.

Specifically, if pair->exit is set (indicating exceeded loop termination),
the test result is ignored, and a TCONF status is returned. This ensures
invalid results are not falsely reported as failures or successes.

  tst_fuzzy_sync.h:654: TINFO: Exceeded execution loops, requesting exit
  /lib/newlib_tests/tst_fuzzy_sync02.c:164: TFAIL: 2| =:15

Signed-off-by: Li Wang <liwang@redhat.com>
---
 lib/newlib_tests/tst_fuzzy_sync01.c | 21 +++++++++++++++++++++
 lib/newlib_tests/tst_fuzzy_sync02.c | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/lib/newlib_tests/tst_fuzzy_sync01.c b/lib/newlib_tests/tst_fuzzy_sync01.c
index d510bd2dd..c8c195859 100644
--- a/lib/newlib_tests/tst_fuzzy_sync01.c
+++ b/lib/newlib_tests/tst_fuzzy_sync01.c
@@ -212,10 +212,31 @@ static void run(unsigned int i)
 
 		if (critical > 100) {
 			tst_fzsync_pair_cleanup(&pair);
+			tst_atomic_store(0, &pair.exit);
 			break;
 		}
 	}
 
+	/*
+	 * If `pair->exit` is true, the test may fail to meet expected
+	 * results due to resource constraints in shared CI environments
+	 * (e.g., GitHub Actions). Limited control over CPU allocation
+	 * can cause delays or interruptions in CPU time slices due to
+	 * contention with other jobs.
+	 *
+	 * Binding the test to a single CPU core (e.g., via `taskset -c 0`)
+	 * can worsen this by increasing contention, leading to performance
+	 * degradation and premature loop termination.
+	 *
+	 * To ensure valid and reliable results in scenarios (e.g., HW, VM, CI),
+	 * it is best to ingore test result when loop termination occurs,
+	 * avoiding unnecessary false postive.
+	 */
+	if (pair.exit) {
+		tst_res(TCONF, "Test may not be able to generate a valid result");
+		return;
+	}
+
 	tst_res(critical > 50 ? TPASS : TFAIL,
 		"acs:%-2d act:%-2d art:%-2d | =:%-4d -:%-4d +:%-4d",
 		a.critical_s, a.critical_t, a.return_t,
diff --git a/lib/newlib_tests/tst_fuzzy_sync02.c b/lib/newlib_tests/tst_fuzzy_sync02.c
index f95394371..ad19a675e 100644
--- a/lib/newlib_tests/tst_fuzzy_sync02.c
+++ b/lib/newlib_tests/tst_fuzzy_sync02.c
@@ -192,10 +192,31 @@ static void run(unsigned int i)
 
 		if (critical > 100) {
 			tst_fzsync_pair_cleanup(&pair);
+			tst_atomic_store(0, &pair.exit);
 			break;
 		}
 	}
 
+	/*
+	 * If `pair->exit` is true, the test may fail to meet expected
+	 * results due to resource constraints in shared CI environments
+	 * (e.g., GitHub Actions). Limited control over CPU allocation
+	 * can cause delays or interruptions in CPU time slices due to
+	 * contention with other jobs.
+	 *
+	 * Binding the test to a single CPU core (e.g., via `taskset -c 0`)
+	 * can worsen this by increasing contention, leading to performance
+	 * degradation and premature loop termination.
+	 *
+	 * To ensure valid and reliable results in scenarios (e.g., HW, VM, CI),
+	 * it is best to ingore test result when loop termination occurs,
+	 * avoiding unnecessary false postive.
+	 */
+	if (pair.exit) {
+		tst_res(TCONF, "Test may not be able to generate a valid result");
+		return;
+	}
+
 	tst_res(critical > 50 ? TPASS : TFAIL, "%d| =:%-4d", i, critical);
 }
 
-- 
2.47.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] newlib_test: enable all tst_fuzzy_sync tests

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---

Notes:
    This patch based on Peter's work:
    Message-ID: <20241210225753.381770-1-pvorel@suse.cz>
    
    CI: https://github.com/wangli5665/ltp/actions/runs/12452324779/

 lib/newlib_tests/runtest.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/newlib_tests/runtest.sh b/lib/newlib_tests/runtest.sh
index 66aaa9361..b00965b16 100755
--- a/lib/newlib_tests/runtest.sh
+++ b/lib/newlib_tests/runtest.sh
@@ -2,7 +2,7 @@
 # Copyright (c) 2021-2024 Petr Vorel <pvorel@suse.cz>
 
 # TODO "unknown failure, exit code": test_assert test08 tst_cgroup01 tst_cgroup02 tst_res_flags variant
-# TODO TFAIL: tst_fuzzy_sync01 tst_fuzzy_sync02 test_macros0[1-6] test23 test26
+# TODO TFAIL: test_macros0[1-6] test23 test26
 # TODO TBROK: test_exec_child test_kconfig01 test_kconfig02 tst_needs_cmds04 tst_needs_cmds05 test_runtime02 test01 test02 test03 test04 test06 test11 test13 test22 test25 tst_safe_fileops
 # TODO TWARN: test_guarded_buf test14 tst_capability01 tst_print_result
 LTP_C_API_TESTS="${LTP_C_API_TESTS:-
@@ -20,6 +20,8 @@ tst_bool_expr
 tst_capability02
 tst_device
 tst_expiration_timer
+tst_fuzzy_sync01
+tst_fuzzy_sync02
 tst_fuzzy_sync03
 tst_needs_cmds0[1-36-8]
 tst_res_hexd
-- 
2.47.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tst_fuzzy_sync: refine code and comment

[Petr Vorel]

Hi Li,

> I just polished some things preventing me from understanding the
> test logic. Hopefully, it makes things easier to read.

LGTM, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tst_fuzzy_sync: refine code and comment

[Petr Vorel]

Hi Li,

would you mind to change before merge trailing "\*/" to "*/"
(removing "/") in both files and extra blank line with "*" in
tst_fuzzy_sync02.c?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] tst_fuzzy_sync: ingnore test result if loop termination occurs

[Petr Vorel]

Hi Li,

> This patch updates the behavior of tst_fuzzy_sync tests to address
> scenarios where test results may be unreliable due to resource constraints.

> Specifically, if pair->exit is set (indicating exceeded loop termination),
> the test result is ignored, and a TCONF status is returned. This ensures
> invalid results are not falsely reported as failures or successes.

>   tst_fuzzy_sync.h:654: TINFO: Exceeded execution loops, requesting exit
>   /lib/newlib_tests/tst_fuzzy_sync02.c:164: TFAIL: 2| =:15

LGTM, good catch.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Below are noted typos, could you please fix them before merge?

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  lib/newlib_tests/tst_fuzzy_sync01.c | 21 +++++++++++++++++++++
>  lib/newlib_tests/tst_fuzzy_sync02.c | 21 +++++++++++++++++++++
>  2 files changed, 42 insertions(+)

> diff --git a/lib/newlib_tests/tst_fuzzy_sync01.c b/lib/newlib_tests/tst_fuzzy_sync01.c
> index d510bd2dd..c8c195859 100644
> --- a/lib/newlib_tests/tst_fuzzy_sync01.c
> +++ b/lib/newlib_tests/tst_fuzzy_sync01.c
> @@ -212,10 +212,31 @@ static void run(unsigned int i)

>  		if (critical > 100) {
>  			tst_fzsync_pair_cleanup(&pair);
> +			tst_atomic_store(0, &pair.exit);
>  			break;
>  		}
>  	}

> +	/*
> +	 * If `pair->exit` is true, the test may fail to meet expected
> +	 * results due to resource constraints in shared CI environments
> +	 * (e.g., GitHub Actions). Limited control over CPU allocation
> +	 * can cause delays or interruptions in CPU time slices due to
> +	 * contention with other jobs.
> +	 *
> +	 * Binding the test to a single CPU core (e.g., via `taskset -c 0`)
> +	 * can worsen this by increasing contention, leading to performance
> +	 * degradation and premature loop termination.
> +	 *
> +	 * To ensure valid and reliable results in scenarios (e.g., HW, VM, CI),
> +	 * it is best to ingore test result when loop termination occurs,
s/ingore/ignore/
> +	 * avoiding unnecessary false postive.
s/postive/positive/

> +	 */
> +	if (pair.exit) {
> +		tst_res(TCONF, "Test may not be able to generate a valid result");
> +		return;
> +	}
> +
>  	tst_res(critical > 50 ? TPASS : TFAIL,
>  		"acs:%-2d act:%-2d art:%-2d | =:%-4d -:%-4d +:%-4d",
>  		a.critical_s, a.critical_t, a.return_t,
> diff --git a/lib/newlib_tests/tst_fuzzy_sync02.c b/lib/newlib_tests/tst_fuzzy_sync02.c
> index f95394371..ad19a675e 100644
> --- a/lib/newlib_tests/tst_fuzzy_sync02.c
> +++ b/lib/newlib_tests/tst_fuzzy_sync02.c
> @@ -192,10 +192,31 @@ static void run(unsigned int i)

>  		if (critical > 100) {
>  			tst_fzsync_pair_cleanup(&pair);
> +			tst_atomic_store(0, &pair.exit);
>  			break;
>  		}
>  	}

> +	/*
> +	 * If `pair->exit` is true, the test may fail to meet expected
> +	 * results due to resource constraints in shared CI environments
> +	 * (e.g., GitHub Actions). Limited control over CPU allocation
> +	 * can cause delays or interruptions in CPU time slices due to
> +	 * contention with other jobs.
> +	 *
> +	 * Binding the test to a single CPU core (e.g., via `taskset -c 0`)
> +	 * can worsen this by increasing contention, leading to performance
> +	 * degradation and premature loop termination.
> +	 *
> +	 * To ensure valid and reliable results in scenarios (e.g., HW, VM, CI),
> +	 * it is best to ingore test result when loop termination occurs,
s/ingore/ignore/
> +	 * avoiding unnecessary false postive.
s/postive/positive/

Also there is original typo from Richie:
s/Emperically/Empirically/

Kind regards,
Petr

> +	 */
> +	if (pair.exit) {
> +		tst_res(TCONF, "Test may not be able to generate a valid result");
> +		return;
> +	}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] newlib_test: enable all tst_fuzzy_sync tests

[Petr Vorel]

Hi Li,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> Notes:
>     This patch based on Peter's work:
>     Message-ID: <20241210225753.381770-1-pvorel@suse.cz>

>     CI: https://github.com/wangli5665/ltp/actions/runs/12452324779/

>  lib/newlib_tests/runtest.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

> diff --git a/lib/newlib_tests/runtest.sh b/lib/newlib_tests/runtest.sh
> index 66aaa9361..b00965b16 100755
> --- a/lib/newlib_tests/runtest.sh
> +++ b/lib/newlib_tests/runtest.sh
> @@ -2,7 +2,7 @@
>  # Copyright (c) 2021-2024 Petr Vorel <pvorel@suse.cz>

>  # TODO "unknown failure, exit code": test_assert test08 tst_cgroup01 tst_cgroup02 tst_res_flags variant
> -# TODO TFAIL: tst_fuzzy_sync01 tst_fuzzy_sync02 test_macros0[1-6] test23 test26
Ah, I should have mark it as tst_fuzzy_sync0[12]

Anyway, feel free to merge before I merge my patchset, I have no problem to
rebase mine.

Kind regards,
Petr

> +# TODO TFAIL: test_macros0[1-6] test23 test26
>  # TODO TBROK: test_exec_child test_kconfig01 test_kconfig02 tst_needs_cmds04 tst_needs_cmds05 test_runtime02 test01 test02 test03 test04 test06 test11 test13 test22 test25 tst_safe_fileops
>  # TODO TWARN: test_guarded_buf test14 tst_capability01 tst_print_result
>  LTP_C_API_TESTS="${LTP_C_API_TESTS:-
> @@ -20,6 +20,8 @@ tst_bool_expr
>  tst_capability02
>  tst_device
>  tst_expiration_timer
> +tst_fuzzy_sync01
> +tst_fuzzy_sync02
>  tst_fuzzy_sync03
>  tst_needs_cmds0[1-36-8]
>  tst_res_hexd

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tst_fuzzy_sync: refine code and comment

[Li Wang]

On Thu, Jan 2, 2025 at 10:41 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> would you mind to change before merge trailing "\*/" to "*/"
> (removing "/") in both files and extra blank line with "*" in
> tst_fuzzy_sync02.c?
>

Thanks for the review, I merge the patch set (with your suggestions).


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp