* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
@ 2021-07-15 10:28 Alexey Kodanev
2021-07-16 7:06 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Alexey Kodanev @ 2021-07-15 10:28 UTC (permalink / raw)
To: ltp
musl doesn't return ESRCH for pthread_kill() if thread id is not found.
POSIX only recommends to return ESRCH, and also says that pthread_kill()
produces undefined behavior if tid lifetime has ended [1].
[1]: https://man7.org/linux/man-pages/man3/pthread_kill.3.html
Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---
testcases/kernel/crypto/af_alg02.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index 31d30777c..0f5793c16 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -60,7 +60,7 @@ static void run(void)
TST_CHECKPOINT_WAIT(0);
- while (pthread_kill(thr, 0) != ESRCH) {
+ while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
if (tst_timeout_remaining() <= 10) {
pthread_cancel(thr);
tst_brk(TBROK,
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-15 10:28 [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill() Alexey Kodanev
@ 2021-07-16 7:06 ` Li Wang
2021-07-16 9:51 ` Petr Vorel
2021-07-16 12:03 ` Alexey Kodanev
0 siblings, 2 replies; 8+ messages in thread
From: Li Wang @ 2021-07-16 7:06 UTC (permalink / raw)
To: ltp
Hi Alexey,
On Thu, Jul 15, 2021 at 6:29 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com>
wrote:
> musl doesn't return ESRCH for pthread_kill() if thread id is not found.
>
> POSIX only recommends to return ESRCH, and also says that pthread_kill()
> produces undefined behavior if tid lifetime has ended [1].
>
> [1]: https://man7.org/linux/man-pages/man3/pthread_kill.3.html
>
> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> ---
> testcases/kernel/crypto/af_alg02.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/crypto/af_alg02.c
> b/testcases/kernel/crypto/af_alg02.c
> index 31d30777c..0f5793c16 100644
> --- a/testcases/kernel/crypto/af_alg02.c
> +++ b/testcases/kernel/crypto/af_alg02.c
> @@ -60,7 +60,7 @@ static void run(void)
>
> TST_CHECKPOINT_WAIT(0);
>
> - while (pthread_kill(thr, 0) != ESRCH) {
> + while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
>
I'm not sure if safe enough to use because it is nonstandard GNU extensions
and the "_np" means nonportable.
Maybe another workaround is to define a volatile flag 'thread_complete',
initialize it to '0' when thread_B starts and reset to '1' while exit, and
just
do a value check in the while loop of thread_A should acquire thread_B
status.
Is this way a bit better?
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210716/67185a7b/attachment.htm>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 7:06 ` Li Wang
@ 2021-07-16 9:51 ` Petr Vorel
2021-07-16 12:12 ` Alexey Kodanev
2021-07-16 12:03 ` Alexey Kodanev
1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-07-16 9:51 UTC (permalink / raw)
To: ltp
> Hi Alexey,
> On Thu, Jul 15, 2021 at 6:29 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> wrote:
> > musl doesn't return ESRCH for pthread_kill() if thread id is not found.
Maybe ask on MUSL mailing list?
> > POSIX only recommends to return ESRCH, and also says that pthread_kill()
> > produces undefined behavior if tid lifetime has ended [1].
> > [1]: https://man7.org/linux/man-pages/man3/pthread_kill.3.html
> > Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> > ---
> > testcases/kernel/crypto/af_alg02.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > diff --git a/testcases/kernel/crypto/af_alg02.c
> > b/testcases/kernel/crypto/af_alg02.c
> > index 31d30777c..0f5793c16 100644
> > --- a/testcases/kernel/crypto/af_alg02.c
> > +++ b/testcases/kernel/crypto/af_alg02.c
> > @@ -60,7 +60,7 @@ static void run(void)
> > TST_CHECKPOINT_WAIT(0);
> > - while (pthread_kill(thr, 0) != ESRCH) {
> > + while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
> I'm not sure if safe enough to use because it is nonstandard GNU extensions
> and the "_np" means nonportable.
Others please double check, but pthread_tryjoin_np() seems to be in uclibc-ng
and musl (+ of course in glibc). It's only missing in bionic (it looks like
people would like to have it [1]).
> Maybe another workaround is to define a volatile flag 'thread_complete',
> initialize it to '0' when thread_B starts and reset to '1' while exit, and
> just
> do a value check in the while loop of thread_A should acquire thread_B
> status.
> Is this way a bit better?
Sounds as reasonable workaround for me.
Kind regards,
Petr
[1] https://github.com/kito-cheng/android-checkpoint/blob/master/bionic/0003-bionic-Implement-pthread_tryjoin_np.patch
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 7:06 ` Li Wang
2021-07-16 9:51 ` Petr Vorel
@ 2021-07-16 12:03 ` Alexey Kodanev
2021-07-16 13:12 ` Li Wang
1 sibling, 1 reply; 8+ messages in thread
From: Alexey Kodanev @ 2021-07-16 12:03 UTC (permalink / raw)
To: ltp
Hi Li,
On 16.07.2021 10:06, Li Wang wrote:
> Hi Alexey,
>
> On Thu, Jul 15, 2021 at 6:29 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>> wrote:
>
> musl doesn't return ESRCH for pthread_kill() if thread id is not found.
>
> POSIX only recommends to return ESRCH, and also says that pthread_kill()
> produces undefined behavior if tid lifetime has ended [1].
>
> [1]: https://man7.org/linux/man-pages/man3/pthread_kill.3.html <https://man7.org/linux/man-pages/man3/pthread_kill.3.html>
>
> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com <mailto:aleksei.kodanev@bell-sw.com>>
> ---
> ?testcases/kernel/crypto/af_alg02.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
> index 31d30777c..0f5793c16 100644
> --- a/testcases/kernel/crypto/af_alg02.c
> +++ b/testcases/kernel/crypto/af_alg02.c
> @@ -60,7 +60,7 @@ static void run(void)
>
> ? ? ? ? TST_CHECKPOINT_WAIT(0);
>
> -? ? ? ?while (pthread_kill(thr, 0) != ESRCH) {
> +? ? ? ?while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
>
>
> I'm not sure ifsafeenough to use because it is nonstandard GNU extensions
> and the "_np" means nonportable.
>
> Maybe another workaround is to define a volatile?flag 'thread_complete',?
> initialize it to '0' when thread_B starts and reset to '1' while exit, and just
> do a value check in the while loop of thread_A should acquire?thread_B status.
> Is this way a bit better?
OK, why not, so something like this:
diff --git a/testcases/kernel/crypto/af_alg02.c b/testcases/kernel/crypto/af_alg02.c
index 0f5793c16..1fe0f3bf0 100644
--- a/testcases/kernel/crypto/af_alg02.c
+++ b/testcases/kernel/crypto/af_alg02.c
@@ -18,11 +18,13 @@
#include "tst_test.h"
#include "tst_af_alg.h"
#include "tst_safe_pthread.h"
+#include "tst_atomic.h"
#include <pthread.h>
#include <errno.h>
#define SALSA20_IV_SIZE 8
#define SALSA20_MIN_KEY_SIZE 16
+static int completed;
static void *verify_encrypt(void *arg)
{
@@ -48,6 +50,8 @@ static void *verify_encrypt(void *arg)
tst_res(TPASS, "Successfully \"encrypted\" an empty message");
else
tst_res(TFAIL, "read() didn't return 0");
+
+ tst_atomic_store(1, &completed);
return arg;
}
@@ -60,7 +64,7 @@ static void run(void)
TST_CHECKPOINT_WAIT(0);
- while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
+ while (!tst_atomic_load(&completed)) {
if (tst_timeout_remaining() <= 10) {
pthread_cancel(thr);
tst_brk(TBROK,
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 9:51 ` Petr Vorel
@ 2021-07-16 12:12 ` Alexey Kodanev
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kodanev @ 2021-07-16 12:12 UTC (permalink / raw)
To: ltp
Hi Petr,
On 16.07.2021 12:51, Petr Vorel wrote:
>> Hi Alexey,
>
>> On Thu, Jul 15, 2021 at 6:29 PM Alexey Kodanev <aleksei.kodanev@bell-sw.com>
>> wrote:
>
>>> musl doesn't return ESRCH for pthread_kill() if thread id is not found.
> Maybe ask on MUSL mailing list?
It's not a musl issue, but I was going to send a few improvements (including
this) when time permits.
>
>>> POSIX only recommends to return ESRCH, and also says that pthread_kill()
>>> produces undefined behavior if tid lifetime has ended [1].
>
>>> [1]: https://man7.org/linux/man-pages/man3/pthread_kill.3.html
>
>>> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
>>> ---
>>> testcases/kernel/crypto/af_alg02.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>>> diff --git a/testcases/kernel/crypto/af_alg02.c
>>> b/testcases/kernel/crypto/af_alg02.c
>>> index 31d30777c..0f5793c16 100644
>>> --- a/testcases/kernel/crypto/af_alg02.c
>>> +++ b/testcases/kernel/crypto/af_alg02.c
>>> @@ -60,7 +60,7 @@ static void run(void)
>
>>> TST_CHECKPOINT_WAIT(0);
>
>>> - while (pthread_kill(thr, 0) != ESRCH) {
>>> + while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
>
>
>> I'm not sure if safe enough to use because it is nonstandard GNU extensions
>> and the "_np" means nonportable.
> Others please double check, but pthread_tryjoin_np() seems to be in uclibc-ng
> and musl (+ of course in glibc). It's only missing in bionic (it looks like
> people would like to have it [1]).
>
Yeah, I think it's quite useful.
>> Maybe another workaround is to define a volatile flag 'thread_complete',
>> initialize it to '0' when thread_B starts and reset to '1' while exit, and
>> just
>> do a value check in the while loop of thread_A should acquire thread_B
>> status.
>> Is this way a bit better?
> Sounds as reasonable workaround for me.
>
> Kind regards,
> Petr
>
> [1] https://github.com/kito-cheng/android-checkpoint/blob/master/bionic/0003-bionic-Implement-pthread_tryjoin_np.patch
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 12:03 ` Alexey Kodanev
@ 2021-07-16 13:12 ` Li Wang
2021-07-16 13:21 ` Li Wang
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2021-07-16 13:12 UTC (permalink / raw)
To: ltp
Alexey Kodanev <aleksei.kodanev@bell-sw.com> wrote:
>
> > Maybe another workaround is to define a volatile flag 'thread_complete',
> > initialize it to '0' when thread_B starts and reset to '1' while exit,
> and just
> > do a value check in the while loop of thread_A should acquire thread_B
> status.
> > Is this way a bit better?
>
> OK, why not, so something like this:
>
> diff --git a/testcases/kernel/crypto/af_alg02.c
> b/testcases/kernel/crypto/af_alg02.c
> index 0f5793c16..1fe0f3bf0 100644
> --- a/testcases/kernel/crypto/af_alg02.c
> +++ b/testcases/kernel/crypto/af_alg02.c
> @@ -18,11 +18,13 @@
> #include "tst_test.h"
> #include "tst_af_alg.h"
> #include "tst_safe_pthread.h"
> +#include "tst_atomic.h"
> #include <pthread.h>
> #include <errno.h>
>
> #define SALSA20_IV_SIZE 8
> #define SALSA20_MIN_KEY_SIZE 16
> +static int completed;
>
> static void *verify_encrypt(void *arg)
> {
> @@ -48,6 +50,8 @@ static void *verify_encrypt(void *arg)
> tst_res(TPASS, "Successfully \"encrypted\" an empty
> message");
> else
> tst_res(TFAIL, "read() didn't return 0");
> +
> + tst_atomic_store(1, &completed);
> return arg;
> }
>
> @@ -60,7 +64,7 @@ static void run(void)
>
> TST_CHECKPOINT_WAIT(0);
>
> - while (pthread_tryjoin_np(thr, NULL) == EBUSY) {
> + while (!tst_atomic_load(&completed)) {
>
+1
The atomic method is quite awesome!
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210716/e43fce96/attachment.htm>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 13:12 ` Li Wang
@ 2021-07-16 13:21 ` Li Wang
2021-07-16 13:35 ` Alexey Kodanev
0 siblings, 1 reply; 8+ messages in thread
From: Li Wang @ 2021-07-16 13:21 UTC (permalink / raw)
To: ltp
> #define SALSA20_IV_SIZE 8
>> #define SALSA20_MIN_KEY_SIZE 16
>> +static int completed;
>>
>> static void *verify_encrypt(void *arg)
>> {
>>
>
But we still need to initialize '0' at the start of thread_B,
in case of test running with '-i xx' parameter. Isn't it?
tst_atomic_store(0, &completed);
--
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20210716/96e883bb/attachment.htm>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill()
2021-07-16 13:21 ` Li Wang
@ 2021-07-16 13:35 ` Alexey Kodanev
0 siblings, 0 replies; 8+ messages in thread
From: Alexey Kodanev @ 2021-07-16 13:35 UTC (permalink / raw)
To: ltp
On 16.07.2021 16:21, Li Wang wrote:
>
> ?#define SALSA20_IV_SIZE? ? ? ?8
> ?#define SALSA20_MIN_KEY_SIZE? 16
> +static int completed;
>
> ?static void *verify_encrypt(void *arg)
> ?{
>
>
> But we still need to initialize?'0' at the start of thread_B,
> in case of test?running with '-i xx' parameter. Isn't it?
>
> ? ???tst_atomic_store(0, &completed);
>
Yeah, right, and that's another reason to use pthread_tryjoin_np() :)
Because otherwise the thread resources not released... anyway we
could add pthread_join in the end of the run()...
> --
> Regards,
> Li Wang
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-16 13:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 10:28 [LTP] [PATCH] crypto/af_alg02: use pthread_tryjoin_np() instead of pthread_kill() Alexey Kodanev
2021-07-16 7:06 ` Li Wang
2021-07-16 9:51 ` Petr Vorel
2021-07-16 12:12 ` Alexey Kodanev
2021-07-16 12:03 ` Alexey Kodanev
2021-07-16 13:12 ` Li Wang
2021-07-16 13:21 ` Li Wang
2021-07-16 13:35 ` Alexey Kodanev
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.