| Message ID | 20170710172459.103764-1-wtc@google.com |
|---|---|
| State | Accepted |
| Commit | 15c41cb6adc4d6720d51c21f8baebebce923b213 |
| Headers | show |
Hi, On Mon, Jul 10, 2017 at 1:24 PM, Wan-Teh Chang <wtc-at-google.com@ffmpeg.org > wrote: > Add the debug_threads boolean field to PerThreadContext. For > PerThreadContext *p, p->debug_threads records whether the > FF_DEBUG_THREADS bit is set in p->avctx->debug, and p->debug_threads and > p->avctx->debug are kept in sync. The debug_threads field is defined as > an atomic_int to allow atomic read by another thread in > ff_thread_await_progress(). > > This fixes the tsan warning that > 2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix: > > WARNING: ThreadSanitizer: data race (pid=452658) > Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write > M248499): > #0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 ( > 5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859) > [..] > Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: write > M248502, write M248500): > #0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 ( > 5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1) > > Signed-off-by: Wan-Teh Chang <wtc@google.com> > --- > libavcodec/pthread_frame.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) I think this looks OK, thanks. I'll leave it out for a day or so for others to review before I merge. Ronald
Hi, On Mon, Jul 10, 2017 at 5:51 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote: > On Mon, Jul 10, 2017 at 1:24 PM, Wan-Teh Chang < > wtc-at-google.com@ffmpeg.org> wrote: > >> Add the debug_threads boolean field to PerThreadContext. For >> PerThreadContext *p, p->debug_threads records whether the >> FF_DEBUG_THREADS bit is set in p->avctx->debug, and p->debug_threads and >> p->avctx->debug are kept in sync. The debug_threads field is defined as >> an atomic_int to allow atomic read by another thread in >> ff_thread_await_progress(). >> >> This fixes the tsan warning that >> 2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix: >> >> WARNING: ThreadSanitizer: data race (pid=452658) >> Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write >> M248499): >> #0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 >> (5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859) >> [..] >> Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: >> write M248502, write M248500): >> #0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 >> (5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1) >> >> Signed-off-by: Wan-Teh Chang <wtc@google.com> >> --- >> libavcodec/pthread_frame.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) > > > I think this looks OK, thanks. I'll leave it out for a day or so for > others to review before I merge. > Pushed. Ronald
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 363b139f71..2cea232a36 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -107,6 +107,8 @@ typedef struct PerThreadContext { int hwaccel_serializing; int async_serializing; + + atomic_int debug_threads; ///< Set if the FF_DEBUG_THREADS option is set. } PerThreadContext; /** @@ -398,6 +400,9 @@ static int submit_packet(PerThreadContext *p, AVCodecContext *user_avctx, pthread_mutex_unlock(&p->mutex); return ret; } + atomic_store_explicit(&p->debug_threads, + (p->avctx->debug & FF_DEBUG_THREADS) != 0, + memory_order_relaxed); release_delayed_buffers(p); @@ -566,7 +571,7 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int field) p = f->owner[field]->internal->thread_ctx; pthread_mutex_lock(&p->progress_mutex); - if (f->owner[field]->debug&FF_DEBUG_THREADS) + if (atomic_load_explicit(&p->debug_threads, memory_order_relaxed)) av_log(f->owner[field], AV_LOG_DEBUG, "%p finished %d field %d\n", progress, n, field); @@ -588,7 +593,7 @@ void ff_thread_await_progress(ThreadFrame *f, int n, int field) p = f->owner[field]->internal->thread_ctx; pthread_mutex_lock(&p->progress_mutex); - if (f->owner[field]->debug&FF_DEBUG_THREADS) + if (atomic_load_explicit(&p->debug_threads, memory_order_relaxed)) av_log(f->owner[field], AV_LOG_DEBUG, "thread awaiting %d field %d from %p\n", n, field, progress); while (atomic_load_explicit(&progress[field], memory_order_relaxed) < n) @@ -823,6 +828,8 @@ int ff_frame_thread_init(AVCodecContext *avctx) if (err) goto error; + atomic_init(&p->debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0); + err = AVERROR(pthread_create(&p->thread, NULL, frame_worker_thread, p)); p->thread_init= !err; if(!p->thread_init)
Add the debug_threads boolean field to PerThreadContext. For PerThreadContext *p, p->debug_threads records whether the FF_DEBUG_THREADS bit is set in p->avctx->debug, and p->debug_threads and p->avctx->debug are kept in sync. The debug_threads field is defined as an atomic_int to allow atomic read by another thread in ff_thread_await_progress(). This fixes the tsan warning that 2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix: WARNING: ThreadSanitizer: data race (pid=452658) Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write M248499): #0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 (5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859) [..] Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: write M248502, write M248500): #0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 (5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1) Signed-off-by: Wan-Teh Chang <wtc@google.com> --- libavcodec/pthread_frame.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)