On 02/10/19 16:58, Torvald Riegel wrote: > This example looks like Dekker synchronization (if I get the intent right).
It is the same pattern. However, one of the two synchronized variables is a counter rather than just a flag. > Two possible implementations of this are either (1) with all memory > accesses having seq-cst MO, or (2) with relaxed-MO accesses and seq-cst > fences on between the store and load on both ends. It's possible to mix > both, but that get's trickier I think. I'd prefer the one with just > fences, just because it's easiest, conceptually. Got it. I'd also prefer the one with just fences, because we only really control one side of the synchronization primitive (ctx_notify_me in my litmus test) and I don't like the idea of forcing seq-cst MO on the other side (bh_scheduled). The performance issue that I mentioned is that x86 doesn't have relaxed fetch and add, so you'd have a redundant fence like this: lock xaddl $2, mem1 mfence ... movl mem1, %r8 (Gory QEMU details however allow us to use relaxed load and store here, because there's only one writer). > It works if you use (1) or (2) consistently. cppmem and the Batty et al. > tech report should give you the gory details. > >> 1) understand why ATOMIC_SEQ_CST is not enough in this case. QEMU code >> seems to be making the same assumptions as Linux about the memory model, >> and this is wrong because QEMU uses C11 atomics if available. >> Fortunately, this kind of synchronization in QEMU is relatively rare and >> only this particular bit seems affected. If there is a fix which stays >> within the C11 memory model, and does not pessimize code on x86, we can >> use it[1] and document the pitfall. > > Using the fences between the store/load pairs in Dekker-like > synchronization should do that, right? It's also relatively easy to deal > with. > >> 2) if there's no way to fix the bug, qemu/atomic.h needs to switch to >> __sync_fetch_and_add and friends. And again, in this case the >> difference between the C11 and Linux/QEMU memory models must be documented. > > I surely not aware of all the constraints here, but I'd be surprised if the > C11 memory model isn't good enough for portable synchronization code (with > the exception of the consume MO minefield, perhaps). This helps a lot already; I'll work on a documentation and code patch. Thanks very much. Paolo >> int main() { >> atomic_int ctx_notify_me = 0; >> atomic_int bh_scheduled = 0; >> {{{ { >> bh_scheduled.store(1, mo_release); >> atomic_thread_fence(mo_seq_cst); >> // must be zero since the bug report shows no notification >> ctx_notify_me.load(mo_relaxed).readsvalue(0); >> } >> ||| { >> ctx_notify_me.store(2, mo_seq_cst); >> r2=bh_scheduled.load(mo_relaxed); >> } >> }}}; >> return 0; >> } ** Changed in: qemu (Ubuntu Disco) Importance: Undecided => Medium ** Changed in: qemu (Ubuntu Bionic) Importance: Undecided => Medium ** Changed in: qemu (Ubuntu Ff-series) Importance: Undecided => Medium -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1805256 Title: qemu-img hangs on rcu_call_ready_event logic in Aarch64 when converting images Status in QEMU: In Progress Status in qemu package in Ubuntu: In Progress Status in qemu source package in Bionic: New Status in qemu source package in Disco: New Status in qemu source package in Eoan: In Progress Status in qemu source package in FF-Series: New Bug description: Command: qemu-img convert -f qcow2 -O qcow2 ./disk01.qcow2 ./output.qcow2 Hangs indefinitely approximately 30% of the runs. ---- Workaround: qemu-img convert -m 1 -f qcow2 -O qcow2 ./disk01.qcow2 ./output.qcow2 Run "qemu-img convert" with "a single coroutine" to avoid this issue. ---- (gdb) thread 1 ... (gdb) bt #0 0x0000ffffbf1ad81c in __GI_ppoll #1 0x0000aaaaaabcf73c in ppoll #2 qemu_poll_ns #3 0x0000aaaaaabd0764 in os_host_main_loop_wait #4 main_loop_wait ... (gdb) thread 2 ... (gdb) bt #0 syscall () #1 0x0000aaaaaabd41cc in qemu_futex_wait #2 qemu_event_wait (ev=ev@entry=0xaaaaaac86ce8 <rcu_call_ready_event>) #3 0x0000aaaaaabed05c in call_rcu_thread #4 0x0000aaaaaabd34c8 in qemu_thread_start #5 0x0000ffffbf25c880 in start_thread #6 0x0000ffffbf1b6b9c in thread_start () (gdb) thread 3 ... (gdb) bt #0 0x0000ffffbf11aa20 in __GI___sigtimedwait #1 0x0000ffffbf2671b4 in __sigwait #2 0x0000aaaaaabd1ddc in sigwait_compat #3 0x0000aaaaaabd34c8 in qemu_thread_start #4 0x0000ffffbf25c880 in start_thread #5 0x0000ffffbf1b6b9c in thread_start ---- (gdb) run Starting program: /usr/bin/qemu-img convert -f qcow2 -O qcow2 ./disk01.ext4.qcow2 ./output.qcow2 [New Thread 0xffffbec5ad90 (LWP 72839)] [New Thread 0xffffbe459d90 (LWP 72840)] [New Thread 0xffffbdb57d90 (LWP 72841)] [New Thread 0xffffacac9d90 (LWP 72859)] [New Thread 0xffffa7ffed90 (LWP 72860)] [New Thread 0xffffa77fdd90 (LWP 72861)] [New Thread 0xffffa6ffcd90 (LWP 72862)] [New Thread 0xffffa67fbd90 (LWP 72863)] [New Thread 0xffffa5ffad90 (LWP 72864)] [Thread 0xffffa5ffad90 (LWP 72864) exited] [Thread 0xffffa6ffcd90 (LWP 72862) exited] [Thread 0xffffa77fdd90 (LWP 72861) exited] [Thread 0xffffbdb57d90 (LWP 72841) exited] [Thread 0xffffa67fbd90 (LWP 72863) exited] [Thread 0xffffacac9d90 (LWP 72859) exited] [Thread 0xffffa7ffed90 (LWP 72860) exited] <HUNG w/ 3 threads in the stack trace showed before> """ All the tasks left are blocked in a system call, so no task left to call qemu_futex_wake() to unblock thread #2 (in futex()), which would unblock thread #1 (doing poll() in a pipe with thread #2). Those 7 threads exit before disk conversion is complete (sometimes in the beginning, sometimes at the end). ---- [ Original Description ] On the HiSilicon D06 system - a 96 core NUMA arm64 box - qemu-img frequently hangs (~50% of the time) with this command: qemu-img convert -f qcow2 -O qcow2 /tmp/cloudimg /tmp/cloudimg2 Where "cloudimg" is a standard qcow2 Ubuntu cloud image. This qcow2->qcow2 conversion happens to be something uvtool does every time it fetches images. Once hung, attaching gdb gives the following backtrace: (gdb) bt #0 0x0000ffffae4f8154 in __GI_ppoll (fds=0xaaaae8a67dc0, nfds=187650274213760, timeout=<optimized out>, timeout@entry=0x0, sigmask=0xffffc123b950) at ../sysdeps/unix/sysv/linux/ppoll.c:39 #1 0x0000aaaabbefaf00 in ppoll (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/aarch64-linux-gnu/bits/poll2.h:77 #2 qemu_poll_ns (fds=<optimized out>, nfds=<optimized out>, timeout=timeout@entry=-1) at util/qemu-timer.c:322 #3 0x0000aaaabbefbf80 in os_host_main_loop_wait (timeout=-1) at util/main-loop.c:233 #4 main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:497 #5 0x0000aaaabbe2aa30 in convert_do_copy (s=0xffffc123bb58) at qemu-img.c:1980 #6 img_convert (argc=<optimized out>, argv=<optimized out>) at qemu-img.c:2456 #7 0x0000aaaabbe2333c in main (argc=7, argv=<optimized out>) at qemu-img.c:4975 Reproduced w/ latest QEMU git (@ 53744e0a182) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1805256/+subscriptions