On Tue, Dec 06, 2016 at 06:16:13PM -0800, Wan-Teh Chang wrote:
> Make the one-time initialization in av_get_cpu_flags() thread-safe. The
> static variable |cpu_flags| in libavutil/cpu.c is read and written using
> normal load and store operations. These are considered as data races.
> The fix is to
Make the one-time initialization in av_get_cpu_flags() thread-safe. The
static variable |cpu_flags| in libavutil/cpu.c is read and written using
normal load and store operations. These are considered as data races.
The fix is to use atomic load and store operations.
The fix can be verified by runn
On Tue, Dec 6, 2016 at 10:41 AM, Wan-Teh Chang wrote:
> Make the one-time initialization in av_get_cpu_flags() thread-safe. The
> static variable |cpu_flags| in libavutil/cpu.c is read and written using
> normal load and store operations. These are considered as data races.
> The fix is to use ato
On Thu, Nov 24, 2016 at 1:56 AM, wm4 wrote:
> On Wed, 23 Nov 2016 11:40:25 -0800
> Wan-Teh Chang wrote:
>
>> On Tue, Nov 22, 2016 at 3:30 PM, wm4 wrote:
>> > On Tue, 22 Nov 2016 23:57:15 +0100
>> > Michael Niedermayer wrote:
>> >
>> >> For example the progress code in the frame threading.
>> >
Make the one-time initialization in av_get_cpu_flags() thread-safe. The
static variable |cpu_flags| in libavutil/cpu.c is read and written using
normal load and store operations. These are considered as data races.
The fix is to use atomic load and store operations.
The fix can be verified by runn
Hi.
Mozilla also fixed some data races in pthread_frames.c which you may want
to look at. It's very similar to problem you mentioned
We made the code conditional on being in an TSan build.
https://dxr.mozilla.org/mozilla-central/source/media/ffvpx/libavcodec/pthread_frame.c#46
Sorry, using the n
On Wed, 23 Nov 2016 11:44:04 -0800
Wan-Teh Chang wrote:
> On Tue, Nov 22, 2016 at 3:32 PM, wm4 wrote:
> >
> > [libav/compat/atomics/] is emulation code for compilers which don't provide
> > C11 atomics.
> > All relevant compilers provide them natively (and presumably implement
> > the weaker se
On Wed, 23 Nov 2016 11:40:25 -0800
Wan-Teh Chang wrote:
> On Tue, Nov 22, 2016 at 3:30 PM, wm4 wrote:
> > On Tue, 22 Nov 2016 23:57:15 +0100
> > Michael Niedermayer wrote:
> >
> >> For example the progress code in the frame threading.
> >
> > Which was recently fixed in Libav AFAIR...
>
On Wed, Nov 23, 2016 at 11:44:04AM -0800, Wan-Teh Chang wrote:
> On Tue, Nov 22, 2016 at 3:32 PM, wm4 wrote:
> >
> > [libav/compat/atomics/] is emulation code for compilers which don't provide
> > C11 atomics.
> > All relevant compilers provide them natively (and presumably implement
> > the weak
On Tue, Nov 22, 2016 at 3:30 PM, wm4 wrote:
> On Tue, 22 Nov 2016 23:57:15 +0100
> Michael Niedermayer wrote:
>
>> For example the progress code in the frame threading.
>
> Which was recently fixed in Libav AFAIR...
You're right. libav/libavcodec/pthread_frame.c has code similar to my
ffmpeg pat
On Tue, Nov 22, 2016 at 3:32 PM, wm4 wrote:
>
> [libav/compat/atomics/] is emulation code for compilers which don't provide
> C11 atomics.
> All relevant compilers provide them natively (and presumably implement
> the weaker semantics).
Thank you for pointing this out.
Do you know when the atom
On Tue, 22 Nov 2016 15:05:36 -0800
Wan-Teh Chang wrote:
> Hi wm4,
>
> On Tue, Nov 22, 2016 at 1:41 PM, wm4 wrote:
> >
> > Again, once the atomics changes in Libav are merged, the avpriv_atomic_
> > additions will have to be deleted, and code using it rewritten.
>
> Thanks for the heads-up. I
On Tue, 22 Nov 2016 23:57:15 +0100
Michael Niedermayer wrote:
> On Tue, Nov 22, 2016 at 02:00:12PM -0800, Wan-Teh Chang wrote:
> > Hi Michael,
> >
> > On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
> > wrote:
> > >
> > > ok, i see th race but do you really need the atomic operations ?
>
Hi Michael,
On Tue, Nov 22, 2016 at 2:57 PM, Michael Niedermayer
wrote:
>
> we have plenty of code that accesses fields without the extra layer
> of weak atomics.
> For example the progress code in the frame threading.
The progress code in the frame threading also has data races as
defined by th
Hi wm4,
On Tue, Nov 22, 2016 at 1:41 PM, wm4 wrote:
>
> Again, once the atomics changes in Libav are merged, the avpriv_atomic_
> additions will have to be deleted, and code using it rewritten.
Thanks for the heads-up. Is there an atomics patch for libav being
reviewed right now?
I inspected th
On Tue, Nov 22, 2016 at 02:00:12PM -0800, Wan-Teh Chang wrote:
> Hi Michael,
>
> On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
> wrote:
> >
> > ok, i see th race but do you really need the atomic operations ?
> > isnt merging the 2 variables into 1 as you do enough ?
> > (i mean the tools
Hi Michael,
On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
wrote:
>
> ok, i see th race but do you really need the atomic operations ?
> isnt merging the 2 variables into 1 as you do enough ?
> (i mean the tools might still complain but would there be an actual
> race remaining?)
The atom
On Tue, 22 Nov 2016 13:36:11 -0800
Wan-Teh Chang wrote:
> Make the one-time initialization in av_get_cpu_flags() thread-safe. The
> static variables |flags| and |checked| in libavutil/cpu.c are read and
> written using normal load and store operations. These are considered as
> data races. The fi
Make the one-time initialization in av_get_cpu_flags() thread-safe. The
static variables |flags| and |checked| in libavutil/cpu.c are read and
written using normal load and store operations. These are considered as
data races. The fix is to use atomic load and store operations. The fix
also elimina
On Tue, Nov 22, 2016 at 11:28:48AM -0800, Wan-Teh Chang wrote:
> Hi Michael,
>
> On Mon, Nov 21, 2016 at 4:35 PM, Michael Niedermayer
> wrote:
> > On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote:
> >> Make the one-time initialization in av_get_cpu_flags() thread-safe.
> >> The fix a
Here is a new version of my patch to address Michael's review comments.
Wan-Teh Chang (1):
Make the one-time initialization in av_get_cpu_flags() thread-safe.
libavutil/Makefile | 1 +
libavutil/atomic.c | 40 ++
libavutil/atomic.h | 34
Hi Michael,
On Mon, Nov 21, 2016 at 4:35 PM, Michael Niedermayer
wrote:
> On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote:
>> Make the one-time initialization in av_get_cpu_flags() thread-safe.
>> The fix assumes -1 is an invalid value for cpu flags.
>
> please explain the bug / rac
On Mon, 21 Nov 2016 15:37:48 -0800
Wan-Teh Chang wrote:
> Hi,
>
> This patch makes the one-time initialization in av_get_cpu_flags()
> thread-safe. The data race was reported by ThreadSanitizer.
>
> Wan-Teh Chang (1):
> avutil: fix data race in av_get_cpu_flags().
>
> libavutil/atomic.c
On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote:
> Make the one-time initialization in av_get_cpu_flags() thread-safe.
> The fix assumes -1 is an invalid value for cpu flags.
please explain the bug / race that occurs and how it is fixed
>
> The fix requires new atomic functions to
Make the one-time initialization in av_get_cpu_flags() thread-safe.
The fix assumes -1 is an invalid value for cpu flags.
The fix requires new atomic functions to get, set, and compare-and-swap
an integer without a memory barrier.
The data race fix is written by Dmitry Vyukov of Google.
Signed-o
Hi,
This patch makes the one-time initialization in av_get_cpu_flags()
thread-safe. The data race was reported by ThreadSanitizer.
Wan-Teh Chang (1):
avutil: fix data race in av_get_cpu_flags().
libavutil/atomic.c | 40
libavutil/atomic.h |
26 matches
Mail list logo
