* [LTP] [PATCH] syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake
@ 2022-09-20 8:28 Jan Stancek
2022-09-20 8:52 ` Andrea Cervesato via ltp
2022-09-20 9:21 ` [LTP] [PATCH v2] syscalls/futex_waitv0[23]: " Jan Stancek
0 siblings, 2 replies; 10+ messages in thread
From: Jan Stancek @ 2022-09-20 8:28 UTC (permalink / raw)
To: ltp
Same patch as for futex_waitv03. TST_THREAD_STATE_WAIT isn't reliable to
tell that it's safe to call futex_wake(). futex_wake() can be called
prematurely and return 0, which leaves other thread timing out.
Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
Also extend timeout to 5 seconds to avoid false positives from systems with
high steal time (e.g. overloaded s390x host).
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
.../kernel/syscalls/futex/futex_waitv02.c | 26 +++++++++----------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
index 0a0e2b62075c..00cf0960808a 100644
--- a/testcases/kernel/syscalls/futex/futex_waitv02.c
+++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
@@ -50,19 +50,20 @@ static void setup(void)
}
}
-static void *threaded(void *arg)
+static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
{
struct futex_test_variants tv = futex_variant();
- int tid = *(int *)arg;
- TST_THREAD_STATE_WAIT(tid, 'S', 0);
-
- TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
+ do {
+ TEST(futex_wake(tv.fntype,
+ (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
1, FUTEX_PRIVATE_FLAG));
- if (TST_RET < 0) {
- tst_brk(TBROK | TTERRNO,
- "futex_wake private returned: %ld", TST_RET);
- }
+ if (TST_RET < 0) {
+ tst_brk(TBROK | TTERRNO,
+ "futex_wake private returned: %ld", TST_RET);
+ }
+ usleep(1000);
+ } while (TST_RET < 1);
return NULL;
}
@@ -70,16 +71,13 @@ static void *threaded(void *arg)
static void run(void)
{
struct timespec to;
- int tid;
pthread_t t;
- tid = tst_syscall(__NR_gettid);
-
- SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
+ SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
/* setting absolute timeout for futex2 */
SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
- to.tv_sec++;
+ to.tv_sec += 5;
TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
if (TST_RET < 0) {
--
2.27.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH] syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 8:28 [LTP] [PATCH] syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake Jan Stancek
@ 2022-09-20 8:52 ` Andrea Cervesato via ltp
2022-09-20 9:21 ` [LTP] [PATCH v2] syscalls/futex_waitv0[23]: " Jan Stancek
1 sibling, 0 replies; 10+ messages in thread
From: Andrea Cervesato via ltp @ 2022-09-20 8:52 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp@lists.linux.it
Hi!
On 9/20/22 10:28, Jan Stancek wrote:
> Same patch as for futex_waitv03. TST_THREAD_STATE_WAIT isn't reliable to
> tell that it's safe to call futex_wake(). futex_wake() can be called
> prematurely and return 0, which leaves other thread timing out.
>
> Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
> Also extend timeout to 5 seconds to avoid false positives from systems with
> high steal time (e.g. overloaded s390x host).
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> .../kernel/syscalls/futex/futex_waitv02.c | 26 +++++++++----------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
> index 0a0e2b62075c..00cf0960808a 100644
> --- a/testcases/kernel/syscalls/futex/futex_waitv02.c
> +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
> @@ -50,19 +50,20 @@ static void setup(void)
> }
> }
>
> -static void *threaded(void *arg)
> +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
> {
> struct futex_test_variants tv = futex_variant();
> - int tid = *(int *)arg;
>
> - TST_THREAD_STATE_WAIT(tid, 'S', 0);
> -
> - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> + do {
> + TEST(futex_wake(tv.fntype,
> + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> 1, FUTEX_PRIVATE_FLAG));
> - if (TST_RET < 0) {
> - tst_brk(TBROK | TTERRNO,
> - "futex_wake private returned: %ld", TST_RET);
> - }
> + if (TST_RET < 0) {
> + tst_brk(TBROK | TTERRNO,
> + "futex_wake private returned: %ld", TST_RET);
> + }
> + usleep(1000);
> + } while (TST_RET < 1);
>
Why not using TST_RETRY_FUNC macro instead?
TST_RETRY_FUNC(futex_wake(tv.fntype, (void
*)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG),
TST_RETVAL_GE0);
> return NULL;
> }
> @@ -70,16 +71,13 @@ static void *threaded(void *arg)
> static void run(void)
> {
> struct timespec to;
> - int tid;
> pthread_t t;
>
> - tid = tst_syscall(__NR_gettid);
> -
> - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
> + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
>
> /* setting absolute timeout for futex2 */
> SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
> - to.tv_sec++;
> + to.tv_sec += 5;
>
> TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
> if (TST_RET < 0) {
--
Regards,
Andrea Cervesato
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 8:28 [LTP] [PATCH] syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake Jan Stancek
2022-09-20 8:52 ` Andrea Cervesato via ltp
@ 2022-09-20 9:21 ` Jan Stancek
2022-09-20 9:53 ` Andrea Cervesato via ltp
1 sibling, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2022-09-20 9:21 UTC (permalink / raw)
To: ltp
TST_THREAD_STATE_WAIT isn't reliable to tell that it's safe to
call futex_wake(). futex_wake() can be called prematurely and
return 0, which leaves other thread timing out.
Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
Also extend timeout to 5 seconds to avoid false positives from systems with
high steal time (e.g. overloaded s390x host).
For futex_waitv03 this replaces while loop with TST_RETRY_FUNC.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
.../kernel/syscalls/futex/futex_waitv02.c | 21 ++++++-------------
.../kernel/syscalls/futex/futex_waitv03.c | 12 +++--------
testcases/kernel/syscalls/futex/futextest.h | 15 +++++++++++++
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
index 0a0e2b62075c..ccea5eb5e745 100644
--- a/testcases/kernel/syscalls/futex/futex_waitv02.c
+++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
@@ -50,19 +50,13 @@ static void setup(void)
}
}
-static void *threaded(void *arg)
+static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
{
struct futex_test_variants tv = futex_variant();
- int tid = *(int *)arg;
- TST_THREAD_STATE_WAIT(tid, 'S', 0);
-
- TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
- 1, FUTEX_PRIVATE_FLAG));
- if (TST_RET < 0) {
- tst_brk(TBROK | TTERRNO,
- "futex_wake private returned: %ld", TST_RET);
- }
+ TST_RETRY_FUNC(futex_wake(tv.fntype,
+ (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
+ 1, FUTEX_PRIVATE_FLAG), futex_waked_someone);
return NULL;
}
@@ -70,16 +64,13 @@ static void *threaded(void *arg)
static void run(void)
{
struct timespec to;
- int tid;
pthread_t t;
- tid = tst_syscall(__NR_gettid);
-
- SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
+ SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
/* setting absolute timeout for futex2 */
SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
- to.tv_sec++;
+ to.tv_sec += 5;
TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
if (TST_RET < 0) {
diff --git a/testcases/kernel/syscalls/futex/futex_waitv03.c b/testcases/kernel/syscalls/futex/futex_waitv03.c
index ee79728474ee..c674f52d8d4c 100644
--- a/testcases/kernel/syscalls/futex/futex_waitv03.c
+++ b/testcases/kernel/syscalls/futex/futex_waitv03.c
@@ -74,15 +74,9 @@ static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
{
struct futex_test_variants tv = futex_variant();
- do {
- TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
- 1, 0));
- if (TST_RET < 0) {
- tst_brk(TBROK | TTERRNO,
- "futex_wake private returned: %ld", TST_RET);
- }
- usleep(1000);
- } while (TST_RET < 1);
+ TST_RETRY_FUNC(futex_wake(tv.fntype,
+ (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
+ 1, 0), futex_waked_someone);
return NULL;
}
diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
index fd10885f3205..515b5102d4fc 100644
--- a/testcases/kernel/syscalls/futex/futextest.h
+++ b/testcases/kernel/syscalls/futex/futextest.h
@@ -277,4 +277,19 @@ futex_set(futex_t *uaddr, u_int32_t newval)
return newval;
}
+/**
+ * futex_waked_someone() - ECHCK func for TST_RETRY_FUNC
+ * @ret: return value of futex_wake
+ *
+ * Return value drives TST_RETRY_FUNC macro.
+ */
+static inline int
+futex_waked_someone(int ret)
+{
+ if (ret < 0)
+ tst_brk(TBROK | TERRNO, "futex_wake returned: %d", ret);
+
+ return ret;
+}
+
#endif /* _FUTEXTEST_H */
--
2.27.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 9:21 ` [LTP] [PATCH v2] syscalls/futex_waitv0[23]: " Jan Stancek
@ 2022-09-20 9:53 ` Andrea Cervesato via ltp
2022-09-20 12:44 ` Jan Stancek
0 siblings, 1 reply; 10+ messages in thread
From: Andrea Cervesato via ltp @ 2022-09-20 9:53 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp@lists.linux.it
Hi!
On 9/20/22 11:21, Jan Stancek wrote:
> TST_THREAD_STATE_WAIT isn't reliable to tell that it's safe to
> call futex_wake(). futex_wake() can be called prematurely and
> return 0, which leaves other thread timing out.
>
> Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
> Also extend timeout to 5 seconds to avoid false positives from systems with
> high steal time (e.g. overloaded s390x host).
>
> For futex_waitv03 this replaces while loop with TST_RETRY_FUNC.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> .../kernel/syscalls/futex/futex_waitv02.c | 21 ++++++-------------
> .../kernel/syscalls/futex/futex_waitv03.c | 12 +++--------
> testcases/kernel/syscalls/futex/futextest.h | 15 +++++++++++++
> 3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
> index 0a0e2b62075c..ccea5eb5e745 100644
> --- a/testcases/kernel/syscalls/futex/futex_waitv02.c
> +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
> @@ -50,19 +50,13 @@ static void setup(void)
> }
> }
>
> -static void *threaded(void *arg)
> +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
> {
> struct futex_test_variants tv = futex_variant();
> - int tid = *(int *)arg;
>
> - TST_THREAD_STATE_WAIT(tid, 'S', 0);
> -
> - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> - 1, FUTEX_PRIVATE_FLAG));
> - if (TST_RET < 0) {
> - tst_brk(TBROK | TTERRNO,
> - "futex_wake private returned: %ld", TST_RET);
> - }
> + TST_RETRY_FUNC(futex_wake(tv.fntype,
> + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> + 1, FUTEX_PRIVATE_FLAG), futex_waked_someone);
Correct way of using TST_RETRY_FUNC is the following:
ret = TST_RETRY_FUNC(futex_wake(tv.fntype, (void
*)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG),
TST_RETVAL_GE0);
if (ret < 0)
tst_brk(TBROK | TTERRNO, "futex_wake private returned: %ld", ret);
>
> return NULL;
> }
> @@ -70,16 +64,13 @@ static void *threaded(void *arg)
> static void run(void)
> {
> struct timespec to;
> - int tid;
> pthread_t t;
>
> - tid = tst_syscall(__NR_gettid);
> -
> - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
> + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
>
> /* setting absolute timeout for futex2 */
> SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
> - to.tv_sec++;
> + to.tv_sec += 5;
>
> TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
> if (TST_RET < 0) {
> diff --git a/testcases/kernel/syscalls/futex/futex_waitv03.c b/testcases/kernel/syscalls/futex/futex_waitv03.c
> index ee79728474ee..c674f52d8d4c 100644
> --- a/testcases/kernel/syscalls/futex/futex_waitv03.c
> +++ b/testcases/kernel/syscalls/futex/futex_waitv03.c
> @@ -74,15 +74,9 @@ static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
> {
> struct futex_test_variants tv = futex_variant();
>
> - do {
> - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> - 1, 0));
> - if (TST_RET < 0) {
> - tst_brk(TBROK | TTERRNO,
> - "futex_wake private returned: %ld", TST_RET);
> - }
> - usleep(1000);
> - } while (TST_RET < 1);
> + TST_RETRY_FUNC(futex_wake(tv.fntype,
> + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> + 1, 0), futex_waked_someone);
>
> return NULL;
> }
> diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
> index fd10885f3205..515b5102d4fc 100644
> --- a/testcases/kernel/syscalls/futex/futextest.h
> +++ b/testcases/kernel/syscalls/futex/futextest.h
> @@ -277,4 +277,19 @@ futex_set(futex_t *uaddr, u_int32_t newval)
> return newval;
> }
>
> +/**
> + * futex_waked_someone() - ECHCK func for TST_RETRY_FUNC
> + * @ret: return value of futex_wake
> + *
> + * Return value drives TST_RETRY_FUNC macro.
> + */
> +static inline int
> +futex_waked_someone(int ret)
> +{
> + if (ret < 0)
> + tst_brk(TBROK | TERRNO, "futex_wake returned: %d", ret);
> +
> + return ret;
> +}
> +
> #endif /* _FUTEXTEST_H */
--
Regards,
Andrea Cervesato
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 9:53 ` Andrea Cervesato via ltp
@ 2022-09-20 12:44 ` Jan Stancek
2022-09-20 13:05 ` Andrea Cervesato via ltp
2022-09-20 21:09 ` Petr Vorel
0 siblings, 2 replies; 10+ messages in thread
From: Jan Stancek @ 2022-09-20 12:44 UTC (permalink / raw)
To: Andrea Cervesato; +Cc: ltp@lists.linux.it
On Tue, Sep 20, 2022 at 11:55 AM Andrea Cervesato
<andrea.cervesato@suse.com> wrote:
>
> Hi!
>
> On 9/20/22 11:21, Jan Stancek wrote:
> > TST_THREAD_STATE_WAIT isn't reliable to tell that it's safe to
> > call futex_wake(). futex_wake() can be called prematurely and
> > return 0, which leaves other thread timing out.
> >
> > Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
> > Also extend timeout to 5 seconds to avoid false positives from systems with
> > high steal time (e.g. overloaded s390x host).
> >
> > For futex_waitv03 this replaces while loop with TST_RETRY_FUNC.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> > .../kernel/syscalls/futex/futex_waitv02.c | 21 ++++++-------------
> > .../kernel/syscalls/futex/futex_waitv03.c | 12 +++--------
> > testcases/kernel/syscalls/futex/futextest.h | 15 +++++++++++++
> > 3 files changed, 24 insertions(+), 24 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
> > index 0a0e2b62075c..ccea5eb5e745 100644
> > --- a/testcases/kernel/syscalls/futex/futex_waitv02.c
> > +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
> > @@ -50,19 +50,13 @@ static void setup(void)
> > }
> > }
> >
> > -static void *threaded(void *arg)
> > +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
> > {
> > struct futex_test_variants tv = futex_variant();
> > - int tid = *(int *)arg;
> >
> > - TST_THREAD_STATE_WAIT(tid, 'S', 0);
> > -
> > - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> > - 1, FUTEX_PRIVATE_FLAG));
> > - if (TST_RET < 0) {
> > - tst_brk(TBROK | TTERRNO,
> > - "futex_wake private returned: %ld", TST_RET);
> > - }
> > + TST_RETRY_FUNC(futex_wake(tv.fntype,
> > + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> > + 1, FUTEX_PRIVATE_FLAG), futex_waked_someone);
>
> Correct way of using TST_RETRY_FUNC is the following:
>
> ret = TST_RETRY_FUNC(futex_wake(tv.fntype, (void
> *)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG),
> TST_RETVAL_GE0);
>
> if (ret < 0)
> tst_brk(TBROK | TTERRNO, "futex_wake private returned: %ld", ret);
This has couple problems:
TST_RETVAL_GE0 aborts retry on futex_wake returning 0.
It won't report a failure (-1), followed by successful call later.
And if the failure (-1) is persistent, it would waste time retrying.
>
> >
> > return NULL;
> > }
> > @@ -70,16 +64,13 @@ static void *threaded(void *arg)
> > static void run(void)
> > {
> > struct timespec to;
> > - int tid;
> > pthread_t t;
> >
> > - tid = tst_syscall(__NR_gettid);
> > -
> > - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
> > + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
> >
> > /* setting absolute timeout for futex2 */
> > SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
> > - to.tv_sec++;
> > + to.tv_sec += 5;
> >
> > TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
> > if (TST_RET < 0) {
> > diff --git a/testcases/kernel/syscalls/futex/futex_waitv03.c b/testcases/kernel/syscalls/futex/futex_waitv03.c
> > index ee79728474ee..c674f52d8d4c 100644
> > --- a/testcases/kernel/syscalls/futex/futex_waitv03.c
> > +++ b/testcases/kernel/syscalls/futex/futex_waitv03.c
> > @@ -74,15 +74,9 @@ static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
> > {
> > struct futex_test_variants tv = futex_variant();
> >
> > - do {
> > - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> > - 1, 0));
> > - if (TST_RET < 0) {
> > - tst_brk(TBROK | TTERRNO,
> > - "futex_wake private returned: %ld", TST_RET);
> > - }
> > - usleep(1000);
> > - } while (TST_RET < 1);
> > + TST_RETRY_FUNC(futex_wake(tv.fntype,
> > + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> > + 1, 0), futex_waked_someone);
> >
> > return NULL;
> > }
> > diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
> > index fd10885f3205..515b5102d4fc 100644
> > --- a/testcases/kernel/syscalls/futex/futextest.h
> > +++ b/testcases/kernel/syscalls/futex/futextest.h
> > @@ -277,4 +277,19 @@ futex_set(futex_t *uaddr, u_int32_t newval)
> > return newval;
> > }
> >
> > +/**
> > + * futex_waked_someone() - ECHCK func for TST_RETRY_FUNC
> > + * @ret: return value of futex_wake
> > + *
> > + * Return value drives TST_RETRY_FUNC macro.
> > + */
> > +static inline int
> > +futex_waked_someone(int ret)
> > +{
> > + if (ret < 0)
> > + tst_brk(TBROK | TERRNO, "futex_wake returned: %d", ret);
> > +
> > + return ret;
> > +}
> > +
> > #endif /* _FUTEXTEST_H */
>
> --
>
> Regards,
> Andrea Cervesato
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 12:44 ` Jan Stancek
@ 2022-09-20 13:05 ` Andrea Cervesato via ltp
2022-09-20 21:09 ` Petr Vorel
1 sibling, 0 replies; 10+ messages in thread
From: Andrea Cervesato via ltp @ 2022-09-20 13:05 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp@lists.linux.it
On 9/20/22 14:44, Jan Stancek wrote:
> On Tue, Sep 20, 2022 at 11:55 AM Andrea Cervesato
> <andrea.cervesato@suse.com> wrote:
>> Hi!
>>
>> On 9/20/22 11:21, Jan Stancek wrote:
>>> TST_THREAD_STATE_WAIT isn't reliable to tell that it's safe to
>>> call futex_wake(). futex_wake() can be called prematurely and
>>> return 0, which leaves other thread timing out.
>>>
>>> Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter.
>>> Also extend timeout to 5 seconds to avoid false positives from systems with
>>> high steal time (e.g. overloaded s390x host).
>>>
>>> For futex_waitv03 this replaces while loop with TST_RETRY_FUNC.
>>>
>>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>>> ---
>>> .../kernel/syscalls/futex/futex_waitv02.c | 21 ++++++-------------
>>> .../kernel/syscalls/futex/futex_waitv03.c | 12 +++--------
>>> testcases/kernel/syscalls/futex/futextest.h | 15 +++++++++++++
>>> 3 files changed, 24 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
>>> index 0a0e2b62075c..ccea5eb5e745 100644
>>> --- a/testcases/kernel/syscalls/futex/futex_waitv02.c
>>> +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
>>> @@ -50,19 +50,13 @@ static void setup(void)
>>> }
>>> }
>>>
>>> -static void *threaded(void *arg)
>>> +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
>>> {
>>> struct futex_test_variants tv = futex_variant();
>>> - int tid = *(int *)arg;
>>>
>>> - TST_THREAD_STATE_WAIT(tid, 'S', 0);
>>> -
>>> - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
>>> - 1, FUTEX_PRIVATE_FLAG));
>>> - if (TST_RET < 0) {
>>> - tst_brk(TBROK | TTERRNO,
>>> - "futex_wake private returned: %ld", TST_RET);
>>> - }
>>> + TST_RETRY_FUNC(futex_wake(tv.fntype,
>>> + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
>>> + 1, FUTEX_PRIVATE_FLAG), futex_waked_someone);
>> Correct way of using TST_RETRY_FUNC is the following:
>>
>> ret = TST_RETRY_FUNC(futex_wake(tv.fntype, (void
>> *)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG),
>> TST_RETVAL_GE0);
>>
>> if (ret < 0)
>> tst_brk(TBROK | TTERRNO, "futex_wake private returned: %ld", ret);
> This has couple problems:
> TST_RETVAL_GE0 aborts retry on futex_wake returning 0.
> It won't report a failure (-1), followed by successful call later.
> And if the failure (-1) is persistent, it would waste time retrying.
I see your point. Ok, so LGTM even without TST_RETRY_FUNC, so we can
allign to futex_waitv03 test.
Acked-by: Andrea Cervesato <andrea.cervesato@suse.de>
>>> return NULL;
>>> }
>>> @@ -70,16 +64,13 @@ static void *threaded(void *arg)
>>> static void run(void)
>>> {
>>> struct timespec to;
>>> - int tid;
>>> pthread_t t;
>>>
>>> - tid = tst_syscall(__NR_gettid);
>>> -
>>> - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid);
>>> + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL);
>>>
>>> /* setting absolute timeout for futex2 */
>>> SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to);
>>> - to.tv_sec++;
>>> + to.tv_sec += 5;
>>>
>>> TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC));
>>> if (TST_RET < 0) {
>>> diff --git a/testcases/kernel/syscalls/futex/futex_waitv03.c b/testcases/kernel/syscalls/futex/futex_waitv03.c
>>> index ee79728474ee..c674f52d8d4c 100644
>>> --- a/testcases/kernel/syscalls/futex/futex_waitv03.c
>>> +++ b/testcases/kernel/syscalls/futex/futex_waitv03.c
>>> @@ -74,15 +74,9 @@ static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
>>> {
>>> struct futex_test_variants tv = futex_variant();
>>>
>>> - do {
>>> - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
>>> - 1, 0));
>>> - if (TST_RET < 0) {
>>> - tst_brk(TBROK | TTERRNO,
>>> - "futex_wake private returned: %ld", TST_RET);
>>> - }
>>> - usleep(1000);
>>> - } while (TST_RET < 1);
>>> + TST_RETRY_FUNC(futex_wake(tv.fntype,
>>> + (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
>>> + 1, 0), futex_waked_someone);
>>>
>>> return NULL;
>>> }
>>> diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
>>> index fd10885f3205..515b5102d4fc 100644
>>> --- a/testcases/kernel/syscalls/futex/futextest.h
>>> +++ b/testcases/kernel/syscalls/futex/futextest.h
>>> @@ -277,4 +277,19 @@ futex_set(futex_t *uaddr, u_int32_t newval)
>>> return newval;
>>> }
>>>
>>> +/**
>>> + * futex_waked_someone() - ECHCK func for TST_RETRY_FUNC
>>> + * @ret: return value of futex_wake
>>> + *
>>> + * Return value drives TST_RETRY_FUNC macro.
>>> + */
>>> +static inline int
>>> +futex_waked_someone(int ret)
>>> +{
>>> + if (ret < 0)
>>> + tst_brk(TBROK | TERRNO, "futex_wake returned: %d", ret);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> #endif /* _FUTEXTEST_H */
>> --
>>
>> Regards,
>> Andrea Cervesato
>>
--
Regards,
Andrea Cervesato
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 12:44 ` Jan Stancek
2022-09-20 13:05 ` Andrea Cervesato via ltp
@ 2022-09-20 21:09 ` Petr Vorel
2022-09-22 9:45 ` Jan Stancek
1 sibling, 1 reply; 10+ messages in thread
From: Petr Vorel @ 2022-09-20 21:09 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp@lists.linux.it
Hi Jan,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-20 21:09 ` Petr Vorel
@ 2022-09-22 9:45 ` Jan Stancek
2022-09-22 9:56 ` Cyril Hrubis
0 siblings, 1 reply; 10+ messages in thread
From: Jan Stancek @ 2022-09-22 9:45 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp@lists.linux.it
On Tue, Sep 20, 2022 at 11:09 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thanks, any concerns pushing this before release? It's race, but not
very frequent one, so it could wait.
>
> Kind regards,
> Petr
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-22 9:45 ` Jan Stancek
@ 2022-09-22 9:56 ` Cyril Hrubis
2022-10-03 10:58 ` Jan Stancek
0 siblings, 1 reply; 10+ messages in thread
From: Cyril Hrubis @ 2022-09-22 9:56 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp@lists.linux.it
Hi!
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Thanks, any concerns pushing this before release? It's race, but not
> very frequent one, so it could wait.
The v2 looks good to me as well.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake
2022-09-22 9:56 ` Cyril Hrubis
@ 2022-10-03 10:58 ` Jan Stancek
0 siblings, 0 replies; 10+ messages in thread
From: Jan Stancek @ 2022-10-03 10:58 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp@lists.linux.it
On Thu, Sep 22, 2022 at 11:56 AM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> >
> > Thanks, any concerns pushing this before release? It's race, but not
> > very frequent one, so it could wait.
>
> The v2 looks good to me as well.
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Pushed.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-10-03 10:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-20 8:28 [LTP] [PATCH] syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake Jan Stancek
2022-09-20 8:52 ` Andrea Cervesato via ltp
2022-09-20 9:21 ` [LTP] [PATCH v2] syscalls/futex_waitv0[23]: " Jan Stancek
2022-09-20 9:53 ` Andrea Cervesato via ltp
2022-09-20 12:44 ` Jan Stancek
2022-09-20 13:05 ` Andrea Cervesato via ltp
2022-09-20 21:09 ` Petr Vorel
2022-09-22 9:45 ` Jan Stancek
2022-09-22 9:56 ` Cyril Hrubis
2022-10-03 10:58 ` Jan Stancek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.