avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
ffmpeg | branch: master | Michael Niedermayer <michaelni@gmx.at> | Sat Jul 13 13:32:39 2013 +0200| [6af8326354ed6c1c68b53b3f2bba6697fb2d3bff] | committer: Michael Niedermayer avcodec/ff_init_vlc_sparse: use a spinlock for thread sync Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=6af8326354ed6c1c68...
libavcodec/bitstream.c | 28 +++++++++++++++++++--------- libavcodec/get_bits.h | 1 + 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c index 3c4d176..aac32a5 100644 --- a/libavcodec/bitstream.c +++ b/libavcodec/bitstream.c @@ -28,6 +28,7 @@ * bitstream api. */ +#include "libavutil/atomic.h" #include "libavutil/avassert.h" #include "avcodec.h" #include "mathops.h" @@ -269,14 +270,17 @@ int ff_init_vlc_sparse(VLC *vlc, int nb_bits, int nb_codes, { VLCcode *buf; int i, j, ret; + void *state; vlc->bits = nb_bits; if (flags & INIT_VLC_USE_NEW_STATIC) { - if (vlc->table_size && vlc->table_size == vlc->table_allocated) { - return 0; - } else if(vlc->table_size) { - abort(); // fatal error, we are called on a partially initialized table + while (state = avpriv_atomic_ptr_cas(&vlc->init_state, NULL, vlc)) { + if (state == vlc + 1) { + av_assert0(vlc->table_size && vlc->table_size == vlc->table_allocated); + return 0; + } } + av_assert0(!vlc->table_size); } else { vlc->table = NULL; vlc->table_allocated = 0; @@ -326,12 +330,18 @@ int ff_init_vlc_sparse(VLC *vlc, int nb_bits, int nb_codes, ret = build_table(vlc, nb_bits, nb_codes, buf, flags); av_free(buf); - if (ret < 0) { - av_freep(&vlc->table); - return ret; + if (flags & INIT_VLC_USE_NEW_STATIC) { + if(vlc->table_size != vlc->table_allocated) + av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", vlc->table_size, vlc->table_allocated); + state = avpriv_atomic_ptr_cas(&vlc->init_state, vlc, vlc+1); + av_assert0(state == vlc); + av_assert0(ret >= 0); + } else { + if (ret < 0) { + av_freep(&vlc->table); + return ret; + } } - if((flags & INIT_VLC_USE_NEW_STATIC) && vlc->table_size != vlc->table_allocated) - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", vlc->table_size, vlc->table_allocated); return 0; } diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h index b6cc75a..3967b49 100644 --- a/libavcodec/get_bits.h +++ b/libavcodec/get_bits.h @@ -64,6 +64,7 @@ typedef struct VLC { int bits; VLC_TYPE (*table)[2]; ///< code, bits int table_size, table_allocated; + void *init_state; } VLC; typedef struct RL_VLC_ELEM {
Michael Niedermayer <git <at> videolan.org> writes:
ffmpeg | branch: master | Michael Niedermayer | Sat Jul 13 13:32:39 2013 +0200| [6af8326354ed6c1c68b53b3f2bba6697fb2d3bff] | committer: Michael Niedermayer
avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
This commit leads to many deadlocks when running fate on AIX PPC64, for example when reading the mp2 file produced by fate-acodec-mp2. The deadlock is not reproducible if I disable all decoders except mp2, it is reproducible (for example) if I enable the mp2 and the mpeg2video decoder. Carl Eugen (gdb) r -i tests/data/fate/acodec-mp2.mp2 Starting program: ffmpeg_g -i tests/data/fate/acodec-mp2.mp2 [New Thread 1] ffmpeg version N-63983-gbc21260 Copyright (c) 2000-2014 the FFmpeg developers built on Jun 15 2014 15:34:47 with gcc 4.8.1 (GCC) configuration: --enable-debug=3 --cc='gcc -maix64' --disable-encoders --enable-encoder=mp2 libavutil 52. 89.100 / 52. 89.100 libavcodec 55. 66.101 / 55. 66.101 libavformat 55. 43.100 / 55. 43.100 libavdevice 55. 13.101 / 55. 13.101 libavfilter 4. 8.100 / 4. 8.100 libswscale 2. 6.100 / 2. 6.100 libswresample 0. 19.100 / 0. 19.100 Program received signal SIGINT, Interrupt. [Switching to Thread 1] 0x00000001002e798c in ff_init_vlc_sparse (vlc=0x11051e5bc <_mpegaudiodecfixed.bss_+255416>, nb_bits=7, nb_codes=512, bits=0xfffffffffffe570, bits_wrap=1, bits_size=1, codes=0xfffffffffffe170, codes_wrap=2, codes_size=2, symbols=0x0, symbols_wrap=0, symbols_size=0, flags=4) at ./libavutil/atomic_gcc.h:57 57 return __sync_val_compare_and_swap(ptr, oldval, newval); (gdb) bt #0 0x00000001002e798c in ff_init_vlc_sparse ( vlc=0x11051e5bc <_mpegaudiodecfixed.bss_+255416>, nb_bits=7, nb_codes=512, bits=0xfffffffffffe570, bits_wrap=1, bits_size=1, codes=0xfffffffffffe170, codes_wrap=2, codes_size=2, symbols=0x0, symbols_wrap=0, symbols_size=0, flags=4) at ./libavutil/atomic_gcc.h:57 #1 0x00000001006cdc94 in decode_init_static () at _start_ :311 #2 0x00000001006ce2fc in decode_init (avctx=<error reading variable>) at _start_ :415 #3 0x0000000100022714 in avcodec_open2 (avctx=0x110655b40, codec=0x110046c50 <ff_mp3_decoder>, options=0x110655fc0) at libavcodec/utils.c:1546 #4 0x00000001000f03a4 in avformat_find_stream_info (ic=0x110654f80, options=0x110655fc0) at libavformat/utils.c:69 #5 0x00000001002cc064 in open_input_file (o=0xffffffffffff0e0, filename=0xffffffffffffc3a "out.mp2") at ffmpeg_opt.c:106 #6 0x00000001002d0fa4 in ffmpeg_parse_options (argc=-3872, argv=0x0) at ffmpeg_opt.c:2645 #7 0x000000010000063c in main (argc=<error reading variable>, argv=<error reading variable>) at _start_ :3807 (gdb) disass $pc-32,$pc+32 Dump of assembler code from 0x1002e796c to 0x1002e79ac: 0x00000001002e796c <ff_init_vlc_sparse+164>: beq 0x1002e79c8 <ff_init_vlc_sparse+256> 0x00000001002e7970 <ff_init_vlc_sparse+168>: addi r8,r3,24 0x00000001002e7974 <ff_init_vlc_sparse+172>: addi r6,r3,32 0x00000001002e7978 <ff_init_vlc_sparse+176>: sync 0x00000001002e797c <ff_init_vlc_sparse+180>: ldarx r9,0,r8 0x00000001002e7980 <ff_init_vlc_sparse+184>: cmpdi cr7,r9,0 0x00000001002e7984 <ff_init_vlc_sparse+188>: bne cr7,0x1002e7990 <ff_init_vlc_sparse+200> 0x00000001002e7988 <ff_init_vlc_sparse+192>: stdcx. r25,0,r8 => 0x00000001002e798c <ff_init_vlc_sparse+196>: bne 0x1002e797c <ff_init_vlc_sparse+180> 0x00000001002e7990 <ff_init_vlc_sparse+200>: isync 0x00000001002e7994 <ff_init_vlc_sparse+204>: beq cr7,0x1002e7a58 <ff_init_vlc_sparse+400> 0x00000001002e7998 <ff_init_vlc_sparse+208>: cmpld cr7,r9,r6 0x00000001002e799c <ff_init_vlc_sparse+212>: bne cr7,0x1002e7978 <ff_init_vlc_sparse+176> 0x00000001002e79a0 <ff_init_vlc_sparse+216>: lwz r9,16(r25) 0x00000001002e79a4 <ff_init_vlc_sparse+220>: cmpwi cr7,r9,0 0x00000001002e79a8 <ff_init_vlc_sparse+224>: beq cr7,0x1002e8028 <ff_init_vlc_sparse+1888> End of assembler dump. (gdb) info register r0 0x4 4 r1 0xfffffffffffb0d0 1152921504606826704 r2 0x1100bc568 4564174184 r3 0x11051e5bc 4568769980 r4 0x7 7 r5 0x200 512 r6 0x11051e5dc 4568770012 r7 0x1 1 r8 0x11051e5d4 4568770004 r9 0x0 0 r10 0x2 2 r11 0x4 4 r12 0x44224242 1143095874 r13 0x110644d20 4569976096 r14 0x4 4 r15 0x0 0 r16 0x4 4 r17 0xfffffffffffe170 1152921504606839152 r18 0x1 1 r19 0xfffffffffffe570 1152921504606840176 r20 0x4 4 r21 0x0 0 r22 0x200 512 r23 0x2 2 r24 0x2 2 r25 0x11051e5bc 4568769980 r26 0x7 7 r27 0x0 0 r28 0x110046ea0 4563693216 r29 0x100a54fb0 4305801136 r30 0x44224242 1143095874 r31 0x0 0 pc 0x1002e798c 0x1002e798c <ff_init_vlc_sparse+196> msr 0xa00000000200d032 11529215046102077490 cr 0x4224242 69354050 lr 0x1006cdc94 0x1006cdc94 <decode_init_static+680> ctr 0x0 0 xer 0x2006c180 537313664
On 16.06.2014, at 00:41, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote:
Michael Niedermayer <git <at> videolan.org> writes:
ffmpeg | branch: master | Michael Niedermayer | Sat Jul 13 13:32:39 2013 +0200| [6af8326354ed6c1c68b53b3f2bba6697fb2d3bff] | committer: Michael Niedermayer
avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
This commit leads to many deadlocks when running fate on AIX PPC64, for example when reading the mp2 file produced by fate-acodec-mp2.
I don't know the exact code that was used, but spinlocks in the traditional sense can only be used when you can _guarantee_ that each thread runs on its own, fully independent CPU. I.e. spinlocks can't be used correctly in user-space, and hangs are the expected outcome when you do it anyway.
Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:
avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
This commit leads to many deadlocks when running fate on AIX PPC64, for example when reading the mp2 file produced by fate-acodec-mp2.
I don't know the exact code that was used, but spinlocks in the traditional sense can only be used when you can _guarantee_ that each thread runs on its own, fully independent CPU.
Some observations: It works fine on the same hardware, the same OS and the same compiler for 32bit. It also works fine on very similar hardware and a different OX for 64bit. Carl Eugen
On Mon, Jun 16, 2014 at 09:44:49AM +0200, Reimar Döffinger wrote:
On 16.06.2014, at 00:41, Carl Eugen Hoyos <cehoyos@ag.or.at> wrote:
Michael Niedermayer <git <at> videolan.org> writes:
ffmpeg | branch: master | Michael Niedermayer | Sat Jul 13 13:32:39 2013 +0200| [6af8326354ed6c1c68b53b3f2bba6697fb2d3bff] | committer: Michael Niedermayer
avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
This commit leads to many deadlocks when running fate on AIX PPC64, for example when reading the mp2 file produced by fate-acodec-mp2.
should be fixed
I don't know the exact code that was used, but spinlocks in the traditional sense can only be used when you can _guarantee_ that each thread runs on its own, fully independent CPU. I.e. spinlocks can't be used correctly in user-space, and hangs are the expected outcome when you do it anyway.
i assumed that all supported OS use modern shedulers that make such deadlocks impossible. the deadlocks on AIX smell like thats not the case also someone should write a test that opens, demuxes and decodes from 2 files at once repeatly so races and deadlocks could be detected Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
Michael Niedermayer <michaelni <at> gmx.at> writes:
avcodec/ff_init_vlc_sparse: use a spinlock for thread sync
This commit leads to many deadlocks when running fate on AIX PPC64, for example when reading the mp2 file produced by fate-acodec-mp2.
should be fixed
fate-acodec-mp2 and many others are fixed, ffprobe still deadlocks on start: (gdb) r Starting program: ffprobe_g [New Thread 1] ^C Program received signal SIGINT, Interrupt. [Switching to Thread 1] 0x00000001000ac980 in avcodec_register (codec=<error reading variable>) at _start_ :268 268 _start_ : A file or directory in the path name does not exist.. (gdb) bt #0 0x00000001000ac980 in avcodec_register (codec=<error reading variable>) at _start_ :268 #1 0x000000010044a6f0 in avcodec_register_all () at libavcodec/allcodecs.c:100 #2 0x00000001002e0d90 in av_register_all () at libavformat/allformats.c:59 #3 0x00000001000005d0 in main (argc=<error reading variable>, argv=<error reading variable>) at _start_ :2919 Carl Eugen
participants (4)
-
Carl Eugen Hoyos -
git＠videolan.org -
Michael Niedermayer -
Reimar Döffinger