Re: [PATCH] erofs-utils: lib: get rid of pthread_cancel() for workqueue

[Yifan Zhao]

LGTM.


Thanks,

Yifan Zhao

On 2024/12/11 10:50, Gao Xiang wrote:

Bionic (Android's libc) does not have pthread_cancel, call
erofs_destroy_workqueue() when initialization fails.

Cc: Kelvin Zhang <zhangkel...@google.com>
Cc: Yifan Zhao <zhaoyi...@sjtu.edu.cn>
Signed-off-by: Gao Xiang <hsiang...@linux.alibaba.com>
---
https://lore.kernel.org/r/20241002174308.2585690-1-zhangkel...@google.com
https://lore.kernel.org/r/20241002174912.42486-1-zhaoyi...@sjtu.edu.cn

  lib/workqueue.c | 63 ++++++++++++++++++++++++-------------------------
  1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/lib/workqueue.c b/lib/workqueue.c
index 47cec9b..840c204 100644
--- a/lib/workqueue.c
+++ b/lib/workqueue.c
@@ -15,9 +15,9 @@ static void *worker_thread(void *arg)
        while (true) {
                pthread_mutex_lock(&wq->lock);

- while (wq->job_count == 0 && !wq->shutdown)

+               while (!wq->job_count && !wq->shutdown)
                        pthread_cond_wait(&wq->cond_empty, &wq->lock);
-               if (wq->job_count == 0 && wq->shutdown) {
+               if (!wq->job_count && wq->shutdown) {
                        pthread_mutex_unlock(&wq->lock);
                        break;
                }
@@ -40,6 +40,29 @@ static void *worker_thread(void *arg)
        return NULL;
  }

+int erofs_destroy_workqueue(struct erofs_workqueue *wq)

+{
+       if (!wq)
+               return -EINVAL;
+
+       pthread_mutex_lock(&wq->lock);
+       wq->shutdown = true;
+       pthread_cond_broadcast(&wq->cond_empty);
+       pthread_mutex_unlock(&wq->lock);
+
+       while (wq->nworker) {
+               int ret = -pthread_join(wq->workers[wq->nworker - 1], NULL);
+
+               if (ret)
+                       return ret;
+               --wq->nworker;
+       }
+       free(wq->workers);
+       pthread_mutex_destroy(&wq->lock);
+       pthread_cond_destroy(&wq->cond_empty);
+       pthread_cond_destroy(&wq->cond_full);
+       return 0;
+}
  int erofs_alloc_workqueue(struct erofs_workqueue *wq, unsigned int nworker,
                          unsigned int max_jobs, erofs_wq_func_t on_start,
                          erofs_wq_func_t on_exit)
@@ -51,7 +74,6 @@ int erofs_alloc_workqueue(struct erofs_workqueue *wq, 
unsigned int nworker,
                return -EINVAL;

wq->head = wq->tail = NULL;

-       wq->nworker = nworker;
        wq->max_jobs = max_jobs;
        wq->job_count = 0;
        wq->shutdown = false;
@@ -67,14 +89,13 @@ int erofs_alloc_workqueue(struct erofs_workqueue *wq, 
unsigned int nworker,

for (i = 0; i < nworker; i++) {

                ret = pthread_create(&wq->workers[i], NULL, worker_thread, wq);
-               if (ret) {
-                       while (i)
-                               pthread_cancel(wq->workers[--i]);
-                       free(wq->workers);
-                       return ret;
-               }
+               if (ret)
+                       break;
        }
-       return 0;
+       wq->nworker = i;
+       if (ret)
+               erofs_destroy_workqueue(wq);
+       return ret;
  }

int erofs_queue_work(struct erofs_workqueue *wq, struct erofs_work *work)

@@ -99,25 +120,3 @@ int erofs_queue_work(struct erofs_workqueue *wq, struct 
erofs_work *work)
        pthread_mutex_unlock(&wq->lock);
        return 0;
  }
-
-int erofs_destroy_workqueue(struct erofs_workqueue *wq)
-{
-       unsigned int i;
-
-       if (!wq)
-               return -EINVAL;
-
-       pthread_mutex_lock(&wq->lock);
-       wq->shutdown = true;
-       pthread_cond_broadcast(&wq->cond_empty);
-       pthread_mutex_unlock(&wq->lock);
-
-       for (i = 0; i < wq->nworker; i++)
-               pthread_join(wq->workers[i], NULL);
-
-       free(wq->workers);
-       pthread_mutex_destroy(&wq->lock);
-       pthread_cond_destroy(&wq->cond_empty);
-       pthread_cond_destroy(&wq->cond_full);
-       return 0;
-}