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

[Gao Xiang]

On 2024/12/11 12:22, Yifan Zhao wrote:
> LGTM.

Thanks, I will add as a r-v-b tag then.

Thanks,
Gao Xiang

> 
> 
> 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 <zhangkelvin at google.com>
>> Cc: Yifan Zhao <zhaoyifan at sjtu.edu.cn>
>> Signed-off-by: Gao Xiang <hsiangkao at linux.alibaba.com>
>> ---
>> https://lore.kernel.org/r/20241002174308.2585690-1-zhangkelvin@google.com
>> https://lore.kernel.org/r/20241002174912.42486-1-zhaoyifan@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;
>> -}