Messages in this thread | | | From | Namhyung Kim <> | Date | Fri, 6 Aug 2021 19:43:09 -0700 | Subject | Re: [RFC PATCH v2 03/10] perf workqueue: add threadpool start and stop functions |
| |
On Fri, Jul 30, 2021 at 8:34 AM Riccardo Mancini <rickyman7@gmail.com> wrote: > > 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/ > > Suggested-by: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com> > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com> > ---
[SNIP] > @@ -93,6 +134,130 @@ static void threadpool_entry__close_pipes(struct threadpool_entry *thread) > } > } > > +/** > + * threadpool__wait_thread - receive ack from thread > + * > + * NB: call only from main thread! > + */ > +static int threadpool__wait_thread(struct threadpool_entry *thread)
If you wanted to differentiate APIs for main thread, I think it's better to pass the pool struct (according to the name) and the index like:
int threadpool__wait_thread(struct threadpool *pool, int idx)
Then it can get a pointer to the entry easily.
> +{ > + int res; > + enum threadpool_msg msg = THREADPOOL_MSG__UNDEFINED; > + > + res = readn(thread->pipes.ack[0], &msg, sizeof(msg)); > + if (res < 0) { > + pr_debug2("threadpool: failed to recv msg from tid=%d: %s\n", > + thread->tid, strerror(errno)); > + return -THREADPOOL_ERROR__READPIPE; > + } > + if (msg != THREADPOOL_MSG__ACK) { > + pr_debug2("threadpool: received unexpected msg from tid=%d: %s\n", > + thread->tid, threadpool_msg_tags[msg]); > + return -THREADPOOL_ERROR__INVALIDMSG; > + } > + > + pr_debug2("threadpool: received ack from tid=%d\n", thread->tid); > + > + return 0; > +} > + > +/** > + * threadpool__terminate_thread - send stop signal to thread and wait for ack > + * > + * NB: call only from main thread! > + */ > +static int threadpool__terminate_thread(struct threadpool_entry *thread)
Ditto.
> +{ > + int res; > + enum threadpool_msg msg = THREADPOOL_MSG__STOP; > + > + res = writen(thread->pipes.cmd[1], &msg, sizeof(msg)); > + if (res < 0) { > + pr_debug2("threadpool: error sending stop msg to tid=%d: %s\n", > + thread->tid, strerror(errno)); > + return -THREADPOOL_ERROR__WRITEPIPE; > + } > + > + return threadpool__wait_thread(thread); > +} > +
[SNIP] > @@ -161,12 +326,30 @@ struct threadpool *threadpool__new(int n_threads) > * > * Buffer size should be at least THREADPOOL_STRERR_BUFSIZE bytes. > */ > -int threadpool__strerror(struct threadpool *pool __maybe_unused, int err, char *buf, size_t size) > +int threadpool__strerror(struct threadpool *pool, int err, char *buf, size_t size) > { > char sbuf[STRERR_BUFSIZE], *emsg; > + const char *status_str, *errno_str; > > - emsg = str_error_r(err, sbuf, sizeof(sbuf)); > - return scnprintf(buf, size, "Error: %s.\n", emsg); > + status_str = IS_ERR_OR_NULL(pool) ? "error" : threadpool_status_tags[pool->status]; > + > + switch (err) { > + case -THREADPOOL_ERROR__SIGPROCMASK: > + case -THREADPOOL_ERROR__READPIPE: > + case -THREADPOOL_ERROR__WRITEPIPE: > + emsg = str_error_r(errno, sbuf, sizeof(sbuf)); > + errno_str = threadpool_errno_str[-err-THREADPOOL_ERROR__OFFSET]; > + return scnprintf(buf, size, "%s: %s.\n", errno_str, emsg); > + case -THREADPOOL_ERROR__INVALIDMSG: > + errno_str = threadpool_errno_str[-err-THREADPOOL_ERROR__OFFSET]; > + return scnprintf(buf, size, "%s.\n", errno_str); > + case -THREADPOOL_ERROR__NOTALLOWED: > + return scnprintf(buf, size, "%s (%s).\n", > + threadpool_errno_str[-err], status_str);
s/-err/-err-THREADPOOL_ERROR__OFFSET/ ?
It'd be nice if you calculate the index once.
> + default: > + emsg = str_error_r(err, sbuf, sizeof(sbuf));
I'm confused whether the 'err' is negative or positive?
Thanks, Namhyung
> + return scnprintf(buf, size, "Error: %s", emsg); > + } > } >
| |