Hi Arnaldo,

On Wed, 2021-07-14 at 12:15 -0300, Arnaldo Carvalho de Melo wrote:

Em Tue, Jul 13, 2021 at 02:11:14PM +0200, Riccardo Mancini escreveu:

quoted

This patch adds the start and stop functions, alongside the thread
function.
Each thread will run until a stop signal is received.
Furthermore, start and stop are added to the test.

Thread management is based on the prototype from Alexey:
https://lore.kernel.org/lkml/cover.1625227739.git.alexey.v.bayduraev@linux.intel.com/ (local)

Suggested-by: Alexey Bayduraev <redacted>
Signed-off-by: Riccardo Mancini <redacted>

<SNIP>

quoted

 
 static int __threadpool__teardown(struct threadpool_struct *pool)
 {
+       int ret;
+
+       ret = stop_threadpool(pool);

        int ret = stop_threadpool(pool);

ok

quoted

+       TEST_ASSERT_VAL("threadpool start failure", ret == 0);
+       TEST_ASSERT_VAL("stopped threadpool is ready",
+                       !threadpool_is_ready(pool));
+
        destroy_threadpool(pool);
 
        return 0;

diff --git a/tools/perf/util/workqueue/threadpool.c

b/tools/perf/util/workqueue/threadpool.c
index 70c67569f956a3e2..f4635ff782b9388e 100644

--- a/tools/perf/util/workqueue/threadpool.c
+++ b/tools/perf/util/workqueue/threadpool.c

@@ -4,12 +4,23 @@

 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
+#include <pthread.h>
+#include <signal.h>
+#include <syscall.h>
 #include "debug.h"
 #include "asm/bug.h"
 #include "threadpool.h"
 
+#ifndef HAVE_GETTID
+static inline pid_t gettid(void)
+{
+       return (pid_t)syscall(__NR_gettid);
+}
+#endif

Isn't this defined elsewhere? Yeah, when we decide to move it to
tools/lib/workqueue/ we'll need it, but for now, reduce patch size.

No, it's just statically defined in tools/perf/jvmti/jvmti_agent.c.
I saw there is a libc_compat.h header in tools/include/tools, I could put this
definition there, and remove the one from jvmti_agent.c.

<SNIP>

quoted

+/**
+ * wait_thread - receive ack from thread
+ *
+ * NB: call only from main thread!
+ */
+static int wait_thread(struct thread_struct *thread)
+{
+       int res;
+       enum thread_msg msg = THREAD_MSG__UNDEFINED;
+
+       res = read(thread->pipes.from[0], &msg, sizeof(msg));

        int res = read(thread->pipes.from[0], &msg, sizeof(msg));

ok

<SNIP>

quoted

+/**
+ * threadpool_thread - function running on thread
+ *
+ * This function waits for a signal from main thread to start executing
+ * a task.
+ * On completion, it will go back to sleep, waiting for another signal.
+ * Signals are delivered through pipes.
+ */
+static void *threadpool_thread(void *args)

   threadpool_function()

 ETOMANY 'thread' in a name.

Agreed :)

<SNIP>

quoted

+/**
+ * start_threadpool - start all threads in the pool.
+ *
+ * The function blocks until all threads are up and running.
+ */
+int start_threadpool(struct threadpool_struct *pool)

int threadpool__start(struct threadpool *pool)

ok

quoted

+{
+       int err;
+
+       if (pool->status != THREADPOOL_STATUS__STOPPED) {
+               pr_err("threadpool: starting not stopped pool\n");
+               return -1;
+       }
+
+       err = __start_threadpool(pool);
+       pool->status = err ? THREADPOOL_STATUS__ERROR :
THREADPOOL_STATUS__READY;
+       return err;
+}
+
+/**
+ * stop_threadpool - stop all threads in the pool.
+ *
+ * This function blocks waiting for ack from all threads.
+ */
+int stop_threadpool(struct threadpool_struct *pool)

int threadpool__stop(struct threadpool *pool)

ok

quoted

+{
+       int t, ret, err = 0;
+
+       if (pool->status != THREADPOOL_STATUS__READY) {
+               pr_err("threadpool: stopping not ready pool\n");
+               return -1;
+       }
+
+       for (t = 0; t < pool->nr_threads; t++) {
+               ret = terminate_thread(&pool->threads[t]);
+               if (ret && !err)
+                       err = -1;
+       }
+
+       pool->status = err ? THREADPOOL_STATUS__ERROR :
THREADPOOL_STATUS__STOPPED;
+
+       return err;
+}
+
+/**
+ * threadpool_is_ready - check if the threads are running
+ */
+bool threadpool_is_ready(struct threadpool_struct *pool)

bool threadpool__is_ready(struct threadpool *pool)

ok

Thanks,
Riccardo

quoted

+{
+       return pool->status == THREADPOOL_STATUS__READY;
+}

diff --git a/tools/perf/util/workqueue/threadpool.h

b/tools/perf/util/workqueue/threadpool.h
index 2b9388c768a0b588..b62cad2b2c5dd331 100644

--- a/tools/perf/util/workqueue/threadpool.h
+++ b/tools/perf/util/workqueue/threadpool.h

@@ -14,6 +14,11 @@ struct task_struct {

 extern struct threadpool_struct *create_threadpool(int n_threads);
 extern void destroy_threadpool(struct threadpool_struct *pool);
 
+extern int start_threadpool(struct threadpool_struct *pool);
+extern int stop_threadpool(struct threadpool_struct *pool);
+
 extern int threadpool_size(struct threadpool_struct *pool);
 
+extern bool threadpool_is_ready(struct threadpool_struct *pool);
+
 #endif /* __WORKQUEUE_THREADPOOL_H */
-- 
2.31.1