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

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

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.)

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".