All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [Patch v4.1] fzsync: revoke thread_b if parent hits accidental break
@ 2019-09-26  9:39 Li Wang
  2019-10-08 12:56 ` Petr Vorel
  0 siblings, 1 reply; 3+ messages in thread
From: Li Wang @ 2019-09-26  9:39 UTC (permalink / raw)
  To: ltp

We shouldn't rely entirely on the pair->exit flag in tst_fzsync_pair_cleanup()
since there is possible to call tst_brk() at anyplace of thread_a, that will
lead to timeout eventually because of thread_b(tst_fzsync_wait_b) fall into
an infinite(no explicit condition to exit) loop.

Thread_a path trace:
  tst_brk()
    cleanup()
      tst_fzsync_pair_cleanup()
        SAFE_PTHREAD_JOIN(pair->thread_b, NULL)
        #Or pthread_cancel(pair->thread_b)

We fix the problem via a way to kill thread_b with pthread_cancel. With new
thread wrapper involves enabling thread cancel at the start of the thread B,
then do asynchronous cancellation in tst_fzsync_pair_cleanup if pair->exit
is not being set to 1.

Workaround: [commit 2e74d996: Check recvmmsg exists before entering fuzzy loop]
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Richard Palethorpe <rpalethorpe@suse.com>
Cc: Cyril Hrubis <chrubis@suse.cz>
Acked-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_fuzzy_sync.h | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index f9a1947c7..c1d0b00f9 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -63,6 +63,7 @@
 #include <time.h>
 #include <math.h>
 #include <stdlib.h>
+#include <pthread.h>
 #include "tst_atomic.h"
 #include "tst_timer.h"
 #include "tst_safe_pthread.h"
@@ -218,12 +219,36 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 static void tst_fzsync_pair_cleanup(struct tst_fzsync_pair *pair)
 {
 	if (pair->thread_b) {
-		tst_atomic_store(1, &pair->exit);
+		/* Revoke thread B if parent hits accidental break */
+		if (!pair->exit) {
+			tst_atomic_store(1, &pair->exit);
+			usleep(100000);
+			pthread_cancel(pair->thread_b);
+		}
 		SAFE_PTHREAD_JOIN(pair->thread_b, NULL);
 		pair->thread_b = 0;
 	}
 }
 
+/** To store the run_b pointer and pass to tst_fzsync_thread_wrapper */
+struct tst_fzsync_run_thread {
+	void *(*func)(void *);
+	void *arg;
+};
+
+/**
+ * Wrap run_b for tst_fzsync_pair_reset to enable pthread cancel
+ * at the start of the thread B.
+ */
+static void *tst_fzsync_thread_wrapper(void *run_thread)
+{
+       struct tst_fzsync_run_thread t = *(struct tst_fzsync_run_thread *)run_thread;
+
+       pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+       pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
+       return t.func(t.arg);
+}
+
 /**
  * Zero some stat fields
  *
@@ -271,8 +296,10 @@ static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,
 	pair->a_cntr = 0;
 	pair->b_cntr = 0;
 	pair->exit = 0;
-	if (run_b)
-		SAFE_PTHREAD_CREATE(&pair->thread_b, 0, run_b, 0);
+	if (run_b) {
+		struct tst_fzsync_run_thread wrap_run_b = {.func = run_b, .arg = NULL};
+		SAFE_PTHREAD_CREATE(&pair->thread_b, 0, tst_fzsync_thread_wrapper, &wrap_run_b);
+	}
 
 	pair->exec_time_start = (float)tst_timeout_remaining();
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [LTP] [Patch v4.1] fzsync: revoke thread_b if parent hits accidental break
  2019-09-26  9:39 [LTP] [Patch v4.1] fzsync: revoke thread_b if parent hits accidental break Li Wang
@ 2019-10-08 12:56 ` Petr Vorel
  2019-10-09  9:25   ` Li Wang
  0 siblings, 1 reply; 3+ messages in thread
From: Petr Vorel @ 2019-10-08 12:56 UTC (permalink / raw)
  To: ltp

Hi LI,

> We shouldn't rely entirely on the pair->exit flag in tst_fzsync_pair_cleanup()
> since there is possible to call tst_brk() at anyplace of thread_a, that will
> lead to timeout eventually because of thread_b(tst_fzsync_wait_b) fall into
> an infinite(no explicit condition to exit) loop.

> Thread_a path trace:
>   tst_brk()
>     cleanup()
>       tst_fzsync_pair_cleanup()
>         SAFE_PTHREAD_JOIN(pair->thread_b, NULL)
>         #Or pthread_cancel(pair->thread_b)

> We fix the problem via a way to kill thread_b with pthread_cancel. With new
> thread wrapper involves enabling thread cancel at the start of the thread B,
> then do asynchronous cancellation in tst_fzsync_pair_cleanup if pair->exit
> is not being set to 1.

> Workaround: [commit 2e74d996: Check recvmmsg exists before entering fuzzy loop]
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Richard Palethorpe <rpalethorpe@suse.com>
> Cc: Cyril Hrubis <chrubis@suse.cz>
> Acked-by: Richard Palethorpe <rpalethorpe@suse.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [LTP] [Patch v4.1] fzsync: revoke thread_b if parent hits accidental break
  2019-10-08 12:56 ` Petr Vorel
@ 2019-10-09  9:25   ` Li Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Li Wang @ 2019-10-09  9:25 UTC (permalink / raw)
  To: ltp

On Tue, Oct 8, 2019 at 8:56 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi LI,
>
> > We shouldn't rely entirely on the pair->exit flag in
> tst_fzsync_pair_cleanup()
> > since there is possible to call tst_brk() at anyplace of thread_a, that
> will
> > lead to timeout eventually because of thread_b(tst_fzsync_wait_b) fall
> into
> > an infinite(no explicit condition to exit) loop.
>
> > Thread_a path trace:
> >   tst_brk()
> >     cleanup()
> >       tst_fzsync_pair_cleanup()
> >         SAFE_PTHREAD_JOIN(pair->thread_b, NULL)
> >         #Or pthread_cancel(pair->thread_b)
>
> > We fix the problem via a way to kill thread_b with pthread_cancel. With
> new
> > thread wrapper involves enabling thread cancel at the start of the
> thread B,
> > then do asynchronous cancellation in tst_fzsync_pair_cleanup if
> pair->exit
> > is not being set to 1.
>
> > Workaround: [commit 2e74d996: Check recvmmsg exists before entering
> fuzzy loop]
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Cc: Richard Palethorpe <rpalethorpe@suse.com>
> > Cc: Cyril Hrubis <chrubis@suse.cz>
> > Acked-by: Richard Palethorpe <rpalethorpe@suse.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>

Pushed. Thanks for the review.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20191009/9fb0a542/attachment.htm>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-10-09  9:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26  9:39 [LTP] [Patch v4.1] fzsync: revoke thread_b if parent hits accidental break Li Wang
2019-10-08 12:56 ` Petr Vorel
2019-10-09  9:25   ` Li Wang

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.