2021-08-31 18:49:25

by Jiri Olsa

[permalink] [raw]
Subject: Re: Possible deadlock errors in tools/perf/builtin-sched.c

On Sat, Aug 28, 2021 at 03:57:17PM +0800, 叶澄锋 wrote:
> Dear developers:
>
> Thank you for your checking.
>
> It seems there are two deadlock errors on the lock
> *sched->work_done_wait_mutex*and*sched->start_work_mutex.*
>
> They are triggered due to one thread(A) runs function *run_one_test* locating
> in a loop and unreleasing the two locks in the*wait_for_tasks*function, and
> another thread(B) runs function *thread_func *acquiring the two locks.
>
> Because the two locks are not properly released in thread A, there will be
> a deadlock problem if thread B acquires the two locks.

hi,
do you have a way to reproduce this?

thanks,
jirka

>
> The related codes are below:
>
> Thread A:
>
> static void create_tasks(struct perf_sched *sched)
> {
> ...;
> err = pthread_mutex_lock(&sched->start_work_mutex);
> ...;
> err = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> }
> static int perf_sched__replay(struct perf_sched *sched)
> {
> ...;
>
> create_tasks(sched);
> printf("------------------------------------------------------------\n");
> for (i = 0; i < sched->replay_repeat; i++)
> run_one_test(sched); // multiple reacquisition on the lock
> sched->work_done_wait_mutex and sched->start_work_mutex
>
> return 0;
> }
>
> static void run_one_test(struct perf_sched *sched)
> {
> ...;
> wait_for_tasks(sched);
> ...;
> }
> static void wait_for_tasks(struct perf_sched *sched)
> {
> ...;
> pthread_mutex_unlock(&sched->work_done_wait_mutex);
>
> ...;
> ret = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> pthread_mutex_unlock(&sched->start_work_mutex);
>
> ...;
>
> ret = pthread_mutex_lock(&sched->start_work_mutex);
> ....;
> }
>
> Thread B:
>
> static void *thread_func(void *ctx)
> {
>
> ...;
> ret = pthread_mutex_lock(&sched->start_work_mutex);
> ...;
> ret = pthread_mutex_unlock(&sched->start_work_mutex);
>
> ...;
>
> ret = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> ret = pthread_mutex_unlock(&sched->work_done_wait_mutex);
> ..;
>
> }
>
>
> Thanks,


2021-09-01 06:22:00

by CAI Yuandao

[permalink] [raw]
Subject: Re: Possible deadlock errors in tools/perf/builtin-sched.c

Hi Jirka,

They were found by our static code analyzer and manually checked as true positives by us. Are they real bugs?

Best,
Ryan

On 1/9/2021, 2:46 AM, "Jiri Olsa" <[email protected]> wrote:

On Sat, Aug 28, 2021 at 03:57:17PM +0800, 叶澄锋 wrote:
> Dear developers:
>
> Thank you for your checking.
>
> It seems there are two deadlock errors on the lock
> *sched->work_done_wait_mutex*and*sched->start_work_mutex.*
>
> They are triggered due to one thread(A) runs function *run_one_test* locating
> in a loop and unreleasing the two locks in the*wait_for_tasks*function, and
> another thread(B) runs function *thread_func *acquiring the two locks.
>
> Because the two locks are not properly released in thread A, there will be
> a deadlock problem if thread B acquires the two locks.

hi,
do you have a way to reproduce this?

thanks,
jirka

>
> The related codes are below:
>
> Thread A:
>
> static void create_tasks(struct perf_sched *sched)
> {
> ...;
> err = pthread_mutex_lock(&sched->start_work_mutex);
> ...;
> err = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> }
> static int perf_sched__replay(struct perf_sched *sched)
> {
> ...;
>
> create_tasks(sched);
> printf("------------------------------------------------------------\n");
> for (i = 0; i < sched->replay_repeat; i++)
> run_one_test(sched); // multiple reacquisition on the lock
> sched->work_done_wait_mutex and sched->start_work_mutex
>
> return 0;
> }
>
> static void run_one_test(struct perf_sched *sched)
> {
> ...;
> wait_for_tasks(sched);
> ...;
> }
> static void wait_for_tasks(struct perf_sched *sched)
> {
> ...;
> pthread_mutex_unlock(&sched->work_done_wait_mutex);
>
> ...;
> ret = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> pthread_mutex_unlock(&sched->start_work_mutex);
>
> ...;
>
> ret = pthread_mutex_lock(&sched->start_work_mutex);
> ....;
> }
>
> Thread B:
>
> static void *thread_func(void *ctx)
> {
>
> ...;
> ret = pthread_mutex_lock(&sched->start_work_mutex);
> ...;
> ret = pthread_mutex_unlock(&sched->start_work_mutex);
>
> ...;
>
> ret = pthread_mutex_lock(&sched->work_done_wait_mutex);
> ...;
> ret = pthread_mutex_unlock(&sched->work_done_wait_mutex);
> ..;
>
> }
>
>
> Thanks,