[FFmpeg-devel] [PATCH] threadprogress: reorder instructions to silence tsan warning.

[Zhao Zhili]

> On Feb 7, 2025, at 19:22, Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
> 
> Ronald S. Bultje:
>> Fixes #11456.
>> ---
>> libavcodec/threadprogress.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/threadprogress.c b/libavcodec/threadprogress.c
>> index 62c4fd898b..aa72ff80e7 100644
>> --- a/libavcodec/threadprogress.c
>> +++ b/libavcodec/threadprogress.c
>> @@ -55,9 +55,8 @@ void ff_thread_progress_report(ThreadProgress *pro, int n)
>>     if (atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
>>         return;
>> 
>> -    atomic_store_explicit(&pro->progress, n, memory_order_release);
>> -
>>     ff_mutex_lock(&pro->progress_mutex);
>> +    atomic_store_explicit(&pro->progress, n, memory_order_release);
>>     ff_cond_broadcast(&pro->progress_cond);
>>     ff_mutex_unlock(&pro->progress_mutex);
>> }
> 
> I don't really understand why this is supposed to fix a race; after all,
> the synchronisation of ff_thread_progress_(report|await) is not supposed
> to be provided by the mutex (which is avoided altogether in the fast
> path in ff_thread_report_await()), but by storing and loading the
> progress variable.
> That's also the reason why I moved this outside of the mutex (compared
> to ff_thread_report_progress(). (This way it is possible for a consumer
> thread to see the new progress value earlier and possibly avoid the
> mutex altogether.)

As I understand it, there is no real race condition, that’s why the patch
says “silence tsan warning”.

I have considered another idea to keep tsan clean and keep the benefit
of set progress earlier: use another non-atomic progress together with
mutex/cond, so atomic and mutex/cond are used separately. Not sure
whether it’s worth the complexity.

> 
> - Andreas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".