diff options
| -rw-r--r-- | block/vdi.c | 11 | ||||
| -rwxr-xr-x | configure | 2 | ||||
| -rw-r--r-- | docs/devel/code-of-conduct.rst | 60 | ||||
| -rw-r--r-- | docs/devel/conflict-resolution.rst | 80 | ||||
| -rw-r--r-- | docs/devel/index.rst | 2 | ||||
| -rw-r--r-- | include/qemu/coroutine.h | 17 | ||||
| -rw-r--r-- | migration/meson.build | 2 | ||||
| -rw-r--r-- | qapi/qom.json | 10 | ||||
| -rw-r--r-- | qom/object_interfaces.c | 1 | ||||
| -rw-r--r-- | replay/replay-events.c | 2 | ||||
| -rw-r--r-- | replay/replay.c | 11 | ||||
| -rw-r--r-- | softmmu/cpu-timers.c | 5 | ||||
| -rw-r--r-- | softmmu/icount.c | 9 | ||||
| -rw-r--r-- | softmmu/timers-state.h | 2 | ||||
| -rw-r--r-- | target/hexagon/meson.build | 36 | ||||
| -rw-r--r-- | target/i386/tcg/translate.c | 6 | ||||
| -rw-r--r-- | target/openrisc/translate.c | 15 | ||||
| -rw-r--r-- | tests/unit/test-coroutine.c | 161 | ||||
| -rw-r--r-- | util/qemu-coroutine-lock.c | 149 |
19 files changed, 471 insertions, 110 deletions
diff --git a/block/vdi.c b/block/vdi.c index 5627e7d764..548f8a057b 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -690,23 +690,26 @@ nonallocating_write: logout("finished data write\n"); if (ret < 0) { + g_free(block); return ret; } if (block) { /* One or more new blocks were allocated. */ - VdiHeader *header = (VdiHeader *) block; + VdiHeader *header; uint8_t *base; uint64_t offset; uint32_t n_sectors; + g_free(block); + header = g_malloc(sizeof(*header)); + logout("now writing modified header\n"); assert(VDI_IS_ALLOCATED(bmap_first)); *header = s->header; vdi_header_to_le(header); - ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); - g_free(block); - block = NULL; + ret = bdrv_pwrite(bs->file, 0, header, sizeof(*header)); + g_free(header); if (ret < 0) { return ret; @@ -365,7 +365,7 @@ tcg_interpreter="false" bigendian="no" mingw32="no" gcov="no" -EXESUF="$default_feature" +EXESUF="" HOST_DSOSUF=".so" modules="no" module_upgrades="no" diff --git a/docs/devel/code-of-conduct.rst b/docs/devel/code-of-conduct.rst new file mode 100644 index 0000000000..277b5250d1 --- /dev/null +++ b/docs/devel/code-of-conduct.rst @@ -0,0 +1,60 @@ +Code of Conduct +=============== + +The QEMU community is made up of a mixture of professionals and +volunteers from all over the world. Diversity is one of our strengths, +but it can also lead to communication issues and unhappiness. +To that end, we have a few ground rules that we ask people to adhere to. + +* Be welcoming. We are committed to making participation in this project + a harassment-free experience for everyone, regardless of level of + experience, gender, gender identity and expression, sexual orientation, + disability, personal appearance, body size, race, ethnicity, age, religion, + or nationality. + +* Be respectful. Not all of us will agree all the time. Disagreements, both + social and technical, happen all the time and the QEMU community is no + exception. When we disagree, we try to understand why. It is important that + we resolve disagreements and differing views constructively. Members of the + QEMU community should be respectful when dealing with other contributors as + well as with people outside the QEMU community and with users of QEMU. + +Harassment and other exclusionary behavior are not acceptable. A community +where people feel uncomfortable or threatened is neither welcoming nor +respectful. Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery + +* Personal attacks + +* Trolling or insulting/derogatory comments + +* Public or private harassment + +* Publishing other's private information, such as physical or electronic + addresses, without explicit permission + +This isn't an exhaustive list of things that you can't do. Rather, take +it in the spirit in which it's intended: a guide to make it easier to +be excellent to each other. + +This code of conduct applies to all spaces managed by the QEMU project. +This includes IRC, the mailing lists, the issue tracker, community +events, and any other forums created by the project team which the +community uses for communication. This code of conduct also applies +outside these spaces, when an individual acts as a representative or a +member of the project or its community. + +By adopting this code of conduct, project maintainers commit themselves +to fairly and consistently applying these principles to every aspect of +managing this project. If you believe someone is violating the code of +conduct, please read the :ref:`conflict-resolution` document for +information about how to proceed. + +Sources +------- + +This document is based on the `Fedora Code of Conduct +<https://fedoraproject.org/code-of-conduct>`__ and the +`Contributor Covenant version 1.3.0 +<https://www.contributor-covenant.org/version/1/3/0/code-of-conduct/>`__. diff --git a/docs/devel/conflict-resolution.rst b/docs/devel/conflict-resolution.rst new file mode 100644 index 0000000000..bb25f61865 --- /dev/null +++ b/docs/devel/conflict-resolution.rst @@ -0,0 +1,80 @@ +.. _conflict-resolution: + +Conflict Resolution Policy +========================== + +Conflicts in the community can take many forms, from someone having a +bad day and using harsh and hurtful language on the mailing list to more +serious code of conduct violations (including sexist/racist statements +or threats of violence), and everything in between. + +For the vast majority of issues, we aim to empower individuals to first +resolve conflicts themselves, asking for help when needed, and only +after that fails to escalate further. This approach gives people more +control over the outcome of their dispute. + +How we resolve conflicts +------------------------ + +If you are experiencing conflict, please consider first addressing the +perceived conflict directly with other involved parties, preferably through +a real-time medium such as IRC. You could also try to get a third-party (e.g. +a mutual friend, and/or someone with background on the issue, but not +involved in the conflict) to intercede or mediate. + +If this fails or if you do not feel comfortable proceeding this way, or +if the problem requires immediate escalation, report the issue to the QEMU +leadership committee by sending an email to qemu@sfconservancy.org, providing +references to the misconduct. +For very urgent topics, you can also inform one or more members through IRC. +The up-to-date list of members is `available on the QEMU wiki +<https://wiki.qemu.org/Conservancy>`__. + +Your report will be treated confidentially by the leadership committee and +not be published without your agreement. The QEMU leadership committee will +then do its best to review the incident in a timely manner, and will either +seek further information, or will make a determination on next steps. + +Remedies +-------- + +Escalating an issue to the QEMU leadership committee may result in actions +impacting one or more involved parties. In the event the leadership +committee has to intervene, here are some of the ways they might respond: + +1. Take no action. For example, if the leadership committee determines + the complaint has not been substantiated or is being made in bad faith, + or if it is deemed to be outside its purview. + +2. A private reprimand, explaining the consequences of continued behavior, + to one or more involved individuals. + +3. A private reprimand and request for a private or public apology + +4. A public reprimand and request for a public apology + +5. A public reprimand plus a mandatory cooling off period. The cooling + off period may require, for example, one or more of the following: + abstaining from maintainer duties; not interacting with people involved, + including unsolicited interaction with those enforcing the guidelines + and interaction on social media; being denied participation to in-person + events. The cooling off period is voluntary but may escalate to a + temporary ban in order to enforce it. + +6. A temporary or permanent ban from some or all current and future QEMU + spaces (mailing lists, IRC, wiki, etc.), possibly including in-person + events. + +In the event of severe harassment, the leadership committee may advise that +the matter be escalated to the relevant local law enforcement agency. It +is however not the role of the leadership committee to initiate contact +with law enforcement on behalf of any of the community members involved +in an incident. + +Sources +------- + +This document was developed based on the `Drupal Conflict Resolution +Policy and Process <https://www.drupal.org/conflict-resolution>`__ +and the `Mozilla Consequence Ladder +<https://github.com/mozilla/diversity/blob/master/code-of-conduct-enforcement/consequence-ladder.md>`__ diff --git a/docs/devel/index.rst b/docs/devel/index.rst index 60039faa68..6cf7e2d233 100644 --- a/docs/devel/index.rst +++ b/docs/devel/index.rst @@ -14,6 +14,8 @@ Contents: :maxdepth: 2 :includehidden: + code-of-conduct + conflict-resolution build-system style kconfig diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h index 84eab6e3bf..ce5b9c6851 100644 --- a/include/qemu/coroutine.h +++ b/include/qemu/coroutine.h @@ -237,11 +237,15 @@ bool qemu_co_enter_next_impl(CoQueue *queue, QemuLockable *lock); bool qemu_co_queue_empty(CoQueue *queue); +typedef struct CoRwTicket CoRwTicket; typedef struct CoRwlock { - int pending_writer; - int reader; CoMutex mutex; - CoQueue queue; + + /* Number of readers, or -1 if owned for writing. */ + int owners; + + /* Waiting coroutines. */ + QSIMPLEQ_HEAD(, CoRwTicket) tickets; } CoRwlock; /** @@ -260,10 +264,9 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock); /** * Write Locks the CoRwlock from a reader. This is a bit more efficient than * @qemu_co_rwlock_unlock followed by a separate @qemu_co_rwlock_wrlock. - * However, if the lock cannot be upgraded immediately, control is transferred - * to the caller of the current coroutine. Also, @qemu_co_rwlock_upgrade - * only overrides CoRwlock fairness if there are no concurrent readers, so - * another writer might run while @qemu_co_rwlock_upgrade blocks. + * Note that if the lock cannot be upgraded immediately, control is transferred + * to the caller of the current coroutine; another writer might run while + * @qemu_co_rwlock_upgrade blocks. */ void qemu_co_rwlock_upgrade(CoRwlock *lock); diff --git a/migration/meson.build b/migration/meson.build index 2cfa8eed72..3ecedce94d 100644 --- a/migration/meson.build +++ b/migration/meson.build @@ -25,7 +25,7 @@ softmmu_ss.add(files( 'savevm.c', 'socket.c', 'tls.c', -)) +), gnutls) softmmu_ss.add(when: ['CONFIG_RDMA', rdma], if_true: files('rdma.c')) softmmu_ss.add(when: 'CONFIG_LIVE_BLOCK_MIGRATION', if_true: files('block.c')) diff --git a/qapi/qom.json b/qapi/qom.json index 2056edc072..db5ac419b1 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -733,8 +733,7 @@ '*policy': 'uint32', '*handle': 'uint32', '*cbitpos': 'uint32', - 'reduced-phys-bits': 'uint32' }, - 'if': 'defined(CONFIG_SEV)' } + 'reduced-phys-bits': 'uint32' } } ## # @ObjectType: @@ -768,14 +767,14 @@ { 'name': 'memory-backend-memfd', 'if': 'defined(CONFIG_LINUX)' }, 'memory-backend-ram', - {'name': 'pef-guest', 'if': 'defined(CONFIG_PSERIES)' }, + 'pef-guest', 'pr-manager-helper', 'rng-builtin', 'rng-egd', 'rng-random', 'secret', 'secret_keyring', - {'name': 'sev-guest', 'if': 'defined(CONFIG_SEV)' }, + 'sev-guest', 's390-pv-guest', 'throttle-group', 'tls-creds-anon', @@ -831,8 +830,7 @@ 'rng-random': 'RngRandomProperties', 'secret': 'SecretProperties', 'secret_keyring': 'SecretKeyringProperties', - 'sev-guest': { 'type': 'SevGuestProperties', - 'if': 'defined(CONFIG_SEV)' }, + 'sev-guest': 'SevGuestProperties', 'throttle-group': 'ThrottleGroupProperties', 'tls-creds-anon': 'TlsCredsAnonProperties', 'tls-creds-psk': 'TlsCredsPskProperties', diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index c3324b0f86..b17aa57de1 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -17,7 +17,6 @@ #include "qemu/qemu-print.h" #include "qapi/opts-visitor.h" #include "qemu/config-file.h" -#include "qemu/qemu-print.h" bool user_creatable_complete(UserCreatable *uc, Error **errp) { diff --git a/replay/replay-events.c b/replay/replay-events.c index a1c6bb934e..15983dd250 100644 --- a/replay/replay-events.c +++ b/replay/replay-events.c @@ -15,6 +15,7 @@ #include "replay-internal.h" #include "block/aio.h" #include "ui/input.h" +#include "hw/core/cpu.h" typedef struct Event { ReplayAsyncEventKind event_kind; @@ -126,6 +127,7 @@ void replay_add_event(ReplayAsyncEventKind event_kind, g_assert(replay_mutex_locked()); QTAILQ_INSERT_TAIL(&events_list, event, events); + qemu_cpu_kick(first_cpu); } void replay_bh_schedule_event(QEMUBH *bh) diff --git a/replay/replay.c b/replay/replay.c index c806fec69a..6df2abc18c 100644 --- a/replay/replay.c +++ b/replay/replay.c @@ -180,12 +180,13 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint) } if (in_checkpoint) { - /* If we are already in checkpoint, then there is no need - for additional synchronization. + /* Recursion occurs when HW event modifies timers. - Timer modification may invoke the checkpoint and - proceed to recursion. */ - return true; + Prevent performing icount warp in this case and + wait for another invocation of the checkpoint. + */ + g_assert(replay_mode == REPLAY_MODE_PLAY); + return false; } in_checkpoint = true; diff --git a/softmmu/cpu-timers.c b/softmmu/cpu-timers.c index cd38595245..34ddfa02f1 100644 --- a/softmmu/cpu-timers.c +++ b/softmmu/cpu-timers.c @@ -188,11 +188,12 @@ static const VMStateDescription icount_vmstate_adjust_timers = { static const VMStateDescription icount_vmstate_shift = { .name = "timer/icount/shift", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .needed = icount_shift_state_needed, .fields = (VMStateField[]) { VMSTATE_INT16(icount_time_shift, TimersState), + VMSTATE_INT64(last_delta, TimersState), VMSTATE_END_OF_LIST() } }; diff --git a/softmmu/icount.c b/softmmu/icount.c index dbcd8c3594..21341a4ce4 100644 --- a/softmmu/icount.c +++ b/softmmu/icount.c @@ -176,9 +176,6 @@ static void icount_adjust(void) int64_t cur_icount; int64_t delta; - /* Protected by TimersState mutex. */ - static int64_t last_delta; - /* If the VM is not running, then do nothing. */ if (!runstate_is_running()) { return; @@ -193,20 +190,20 @@ static void icount_adjust(void) delta = cur_icount - cur_time; /* FIXME: This is a very crude algorithm, somewhat prone to oscillation. */ if (delta > 0 - && last_delta + ICOUNT_WOBBLE < delta * 2 + && timers_state.last_delta + ICOUNT_WOBBLE < delta * 2 && timers_state.icount_time_shift > 0) { /* The guest is getting too far ahead. Slow time down. */ qatomic_set(&timers_state.icount_time_shift, timers_state.icount_time_shift - 1); } if (delta < 0 - && last_delta - ICOUNT_WOBBLE > delta * 2 + && timers_state.last_delta - ICOUNT_WOBBLE > delta * 2 && timers_state.icount_time_shift < MAX_ICOUNT_SHIFT) { /* The guest is getting too far behind. Speed time up. */ qatomic_set(&timers_state.icount_time_shift, timers_state.icount_time_shift + 1); } - last_delta = delta; + timers_state.last_delta = delta; qatomic_set_i64(&timers_state.qemu_icount_bias, cur_icount - (timers_state.qemu_icount << timers_state.icount_time_shift)); diff --git a/softmmu/timers-state.h b/softmmu/timers-state.h index db4e60f18f..8c262ce139 100644 --- a/softmmu/timers-state.h +++ b/softmmu/timers-state.h @@ -43,6 +43,8 @@ typedef struct TimersState { /* Conversion factor from emulated instructions to virtual clock ticks. */ int16_t icount_time_shift; + /* Icount delta used for shift auto adjust. */ + int64_t last_delta; /* Compensate for varying guest execution speed. */ int64_t qemu_icount_bias; diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build index 15318a6fa7..bb0b4fb621 100644 --- a/target/hexagon/meson.build +++ b/target/hexagon/meson.build @@ -33,8 +33,7 @@ gen_semantics = executable( semantics_generated = custom_target( 'semantics_generated.pyinc', output: 'semantics_generated.pyinc', - input: gen_semantics, - command: ['@INPUT@', '@OUTPUT@'], + command: [gen_semantics, '@OUTPUT@'], ) hexagon_ss.add(semantics_generated) @@ -54,90 +53,81 @@ hexagon_ss.add(semantics_generated) shortcode_generated = custom_target( 'shortcode_generated.h.inc', output: 'shortcode_generated.h.inc', - input: 'gen_shortcode.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_shortcode.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(shortcode_generated) helper_protos_generated = custom_target( 'helper_protos_generated.h.inc', output: 'helper_protos_generated.h.inc', - input: 'gen_helper_protos.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def, gen_tcg_h], - command: [python, '@INPUT@', semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], + command: [python, files('gen_helper_protos.py'), semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], ) hexagon_ss.add(helper_protos_generated) tcg_funcs_generated = custom_target( 'tcg_funcs_generated.c.inc', output: 'tcg_funcs_generated.c.inc', - input: 'gen_tcg_funcs.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def, gen_tcg_h], - command: [python, '@INPUT@', semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], + command: [python, files('gen_tcg_funcs.py'), semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], ) hexagon_ss.add(tcg_funcs_generated) tcg_func_table_generated = custom_target( 'tcg_func_table_generated.c.inc', output: 'tcg_func_table_generated.c.inc', - input: 'gen_tcg_func_table.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_tcg_func_table.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(tcg_func_table_generated) helper_funcs_generated = custom_target( 'helper_funcs_generated.c.inc', output: 'helper_funcs_generated.c.inc', - input: 'gen_helper_funcs.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def, gen_tcg_h], - command: [python, '@INPUT@', semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], + command: [python, files('gen_helper_funcs.py'), semantics_generated, attribs_def, gen_tcg_h, '@OUTPUT@'], ) hexagon_ss.add(helper_funcs_generated) printinsn_generated = custom_target( 'printinsn_generated.h.inc', output: 'printinsn_generated.h.inc', - input: 'gen_printinsn.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_printinsn.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(printinsn_generated) op_regs_generated = custom_target( 'op_regs_generated.h.inc', output: 'op_regs_generated.h.inc', - input: 'gen_op_regs.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_op_regs.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(op_regs_generated) op_attribs_generated = custom_target( 'op_attribs_generated.h.inc', output: 'op_attribs_generated.h.inc', - input: 'gen_op_attribs.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_op_attribs.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(op_attribs_generated) opcodes_def_generated = custom_target( 'opcodes_def_generated.h.inc', output: 'opcodes_def_generated.h.inc', - input: 'gen_opcodes_def.py', depends: [semantics_generated], depend_files: [hex_common_py, attribs_def], - command: [python, '@INPUT@', semantics_generated, attribs_def, '@OUTPUT@'], + command: [python, files('gen_opcodes_def.py'), semantics_generated, attribs_def, '@OUTPUT@'], ) hexagon_ss.add(opcodes_def_generated) @@ -154,8 +144,7 @@ gen_dectree_import = executable( iset_py = custom_target( 'iset.py', output: 'iset.py', - input: gen_dectree_import, - command: ['@INPUT@', '@OUTPUT@'], + command: [gen_dectree_import, '@OUTPUT@'], ) hexagon_ss.add(iset_py) @@ -166,9 +155,8 @@ hexagon_ss.add(iset_py) dectree_generated = custom_target( 'dectree_generated.h.inc', output: 'dectree_generated.h.inc', - input: 'dectree.py', depends: [iset_py], - command: ['PYTHONPATH=' + meson.current_build_dir(), '@INPUT@', '@OUTPUT@'], + command: ['env', 'PYTHONPATH=' + meson.current_build_dir(), files('dectree.py'), '@OUTPUT@'], ) hexagon_ss.add(dectree_generated) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index af1faf9342..880bc45561 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -5061,6 +5061,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) gen_jr(s, s->T0); break; case 3: /* lcall Ev */ + if (mod == 3) { + goto illegal_op; + } gen_op_ld_v(s, ot, s->T1, s->A0); gen_add_A0_im(s, 1 << ot); gen_op_ld_v(s, MO_16, s->T0, s->A0); @@ -5088,6 +5091,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) gen_jr(s, s->T0); break; case 5: /* ljmp Ev */ + if (mod == 3) { + goto illegal_op; + } gen_op_ld_v(s, ot, s->T1, s->A0); gen_add_A0_im(s, 1 << ot); gen_op_ld_v(s, MO_16, s->T0, s->A0); diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index c6dce879f1..a9c81f8bd5 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -884,6 +884,18 @@ static bool trans_l_mfspr(DisasContext *dc, arg_l_mfspr *a) gen_illegal_exception(dc); } else { TCGv spr = tcg_temp_new(); + + if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) { + gen_io_start(); + if (dc->delayed_branch) { + tcg_gen_mov_tl(cpu_pc, jmp_pc); + tcg_gen_discard_tl(jmp_pc); + } else { + tcg_gen_movi_tl(cpu_pc, dc->base.pc_next + 4); + } + dc->base.is_jmp = DISAS_EXIT; + } + tcg_gen_ori_tl(spr, cpu_R(dc, a->a), a->k); gen_helper_mfspr(cpu_R(dc, a->d), cpu_env, cpu_R(dc, a->d), spr); tcg_temp_free(spr); @@ -898,6 +910,9 @@ static bool trans_l_mtspr(DisasContext *dc, arg_l_mtspr *a) } else { TCGv spr; + if (tb_cflags(dc->base.tb) & CF_USE_ICOUNT) { + gen_io_start(); + } /* For SR, we will need to exit the TB to recognize the new * exception state. For NPC, in theory this counts as a branch * (although the SPR only exists for use by an ICE). Save all diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c index e946d93a65..aa77a3bcb3 100644 --- a/tests/unit/test-coroutine.c +++ b/tests/unit/test-coroutine.c @@ -264,6 +264,165 @@ static void test_co_mutex_lockable(void) g_assert(QEMU_MAKE_LOCKABLE(null_pointer) == NULL); } +static CoRwlock rwlock; + +/* Test that readers are properly sent back to the queue when upgrading, + * even if they are the sole readers. The test scenario is as follows: + * + * + * | c1 | c2 | + * |--------------+------------+ + * | rdlock | | + * | yield | | + * | | wrlock | + * | | <queued> | + * | upgrade | | + * | <queued> | <dequeued> | + * | | unlock | + * | <dequeued> | | + * | unlock | | + */ + +static void coroutine_fn rwlock_yield_upgrade(void *opaque) +{ + qemu_co_rwlock_rdlock(&rwlock); + qemu_coroutine_yield(); + + qemu_co_rwlock_upgrade(&rwlock); + qemu_co_rwlock_unlock(&rwlock); + + *(bool *)opaque = true; +} + +static void coroutine_fn rwlock_wrlock_yield(void *opaque) +{ + qemu_co_rwlock_wrlock(&rwlock); + qemu_coroutine_yield(); + + qemu_co_rwlock_unlock(&rwlock); + *(bool *)opaque = true; +} + +static void test_co_rwlock_upgrade(void) +{ + bool c1_done = false; + bool c2_done = false; + Coroutine *c1, *c2; + + qemu_co_rwlock_init(&rwlock); + c1 = qemu_coroutine_create(rwlock_yield_upgrade, &c1_done); + c2 = qemu_coroutine_create(rwlock_wrlock_yield, &c2_done); + + qemu_coroutine_enter(c1); + qemu_coroutine_enter(c2); + + /* c1 now should go to sleep. */ + qemu_coroutine_enter(c1); + g_assert(!c1_done); + + qemu_coroutine_enter(c2); + g_assert(c1_done); + g_assert(c2_done); +} + +static void coroutine_fn rwlock_rdlock_yield(void *opaque) +{ + qemu_co_rwlock_rdlock(&rwlock); + qemu_coroutine_yield(); + + qemu_co_rwlock_unlock(&rwlock); + qemu_coroutine_yield(); + + *(bool *)opaque = true; +} + +static void coroutine_fn rwlock_wrlock_downgrade(void *opaque) +{ + qemu_co_rwlock_wrlock(&rwlock); + + qemu_co_rwlock_downgrade(&rwlock); + qemu_co_rwlock_unlock(&rwlock); + *(bool *)opaque = true; +} + +static void coroutine_fn rwlock_rdlock(void *opaque) +{ + qemu_co_rwlock_rdlock(&rwlock); + + qemu_co_rwlock_unlock(&rwlock); + *(bool *)opaque = true; +} + +static void coroutine_fn rwlock_wrlock(void *opaque) +{ + qemu_co_rwlock_wrlock(&rwlock); + + qemu_co_rwlock_unlock(&rwlock); + *(bool *)opaque = true; +} + +/* + * Check that downgrading a reader-writer lock does not cause a hang. + * + * Four coroutines are used to produce a situation where there are + * both reader and writer hopefuls waiting to acquire an rwlock that + * is held by a reader. + * + * The correct sequence of operations we aim to provoke can be + * represented as: + * + * | c1 | c2 | c3 | c4 | + * |--------+------------+------------+------------| + * | rdlock | | | | + * | yield | | | | + * | | wrlock | | | + * | | <queued> | | | + * | | | rdlock | | + * | | | <queued> | | + * | | | | wrlock | + * | | | | <queued> | + * | unlock | | | | + * | yield | | | | + * | | <dequeued> | | | + * | | downgrade | | | + * | | | <dequeued> | | + * | | | unlock | | + * | | ... | | | + * | | unlock | | | + * | | | | <dequeued> | + * | | | | unlock | + */ +static void test_co_rwlock_downgrade(void) +{ + bool c1_done = false; + bool c2_done = false; + bool c3_done = false; + bool c4_done = false; + Coroutine *c1, *c2, *c3, *c4; + + qemu_co_rwlock_init(&rwlock); + + c1 = qemu_coroutine_create(rwlock_rdlock_yield, &c1_done); + c2 = qemu_coroutine_create(rwlock_wrlock_downgrade, &c2_done); + c3 = qemu_coroutine_create(rwlock_rdlock, &c3_done); + c4 = qemu_coroutine_create(rwlock_wrlock, &c4_done); + + qemu_coroutine_enter(c1); + qemu_coroutine_enter(c2); + qemu_coroutine_enter(c3); + qemu_coroutine_enter(c4); + + qemu_coroutine_enter(c1); + + g_assert(c2_done); + g_assert(c3_done); + g_assert(c4_done); + + qemu_coroutine_enter(c1); + + g_assert(c1_done); +} + /* * Check that creation, enter, and return work */ @@ -501,6 +660,8 @@ int main(int argc, char **argv) g_test_add_func("/basic/order", test_order); g_test_add_func("/locking/co-mutex", test_co_mutex); g_test_add_func("/locking/co-mutex/lockable", test_co_mutex_lockable); + g_test_add_func("/locking/co-rwlock/upgrade", test_co_rwlock_upgrade); + g_test_add_func("/locking/co-rwlock/downgrade", test_co_rwlock_downgrade); if (g_test_perf()) { g_test_add_func("/perf/lifecycle", perf_lifecycle); g_test_add_func("/perf/nesting", perf_nesting); diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c index 5816bf8900..2669403839 100644 --- a/util/qemu-coroutine-lock.c +++ b/util/qemu-coroutine-lock.c @@ -204,7 +204,6 @@ static void coroutine_fn qemu_co_mutex_lock_slowpath(AioContext *ctx, unsigned old_handoff; trace_qemu_co_mutex_lock_entry(mutex, self); - w.co = self; push_waiter(mutex, &w); /* This is the "Responsibility Hand-Off" protocol; a lock() picks from @@ -328,11 +327,51 @@ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex) trace_qemu_co_mutex_unlock_return(mutex, self); } +struct CoRwTicket { + bool read; + Coroutine *co; + QSIMPLEQ_ENTRY(CoRwTicket) next; +}; + void qemu_co_rwlock_init(CoRwlock *lock) { - memset(lock, 0, sizeof(*lock)); - qemu_co_queue_init(&lock->queue); qemu_co_mutex_init(&lock->mutex); + lock->owners = 0; + QSIMPLEQ_INIT(&lock->tickets); +} + +/* Releases the internal CoMutex. */ +static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock) +{ + CoRwTicket *tkt = QSIMPLEQ_FIRST(&lock->tickets); + Coroutine *co = NULL; + + /* + * Setting lock->owners here prevents rdlock and wrlock from + * sneaking in between unlock and wake. + */ + + if (tkt) { + if (tkt->read) { + if (lock->owners >= 0) { + lock->owners++; + co = tkt->co; + } + } else { + if (lock->owners == 0) { + lock->owners = -1; + co = tkt->co; + } + } + } + + if (co) { + QSIMPLEQ_REMOVE_HEAD(&lock->tickets, next); + qemu_co_mutex_unlock(&lock->mutex); + aio_co_wake(co); + } else { + qemu_co_mutex_unlock(&lock->mutex); + } } void qemu_co_rwlock_rdlock(CoRwlock *lock) @@ -341,13 +380,22 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock) qemu_co_mutex_lock(&lock->mutex); /* For fairness, wait if a writer is in line. */ - while (lock->pending_writer) { - qemu_co_queue_wait(&lock->queue, &lock->mutex); + if (lock->owners == 0 || (lock->owners > 0 && QSIMPLEQ_EMPTY(&lock->tickets))) { + lock->owners++; + qemu_co_mutex_unlock(&lock->mutex); + } else { + CoRwTicket my_ticket = { true, self }; + + QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next); + qemu_co_mutex_unlock(&lock->mutex); + qemu_coroutine_yield(); + assert(lock->owners >= 1); + + /* Possibly wake another reader, which will wake the next in line. */ + qemu_co_mutex_lock(&lock->mutex); + qemu_co_rwlock_maybe_wake_one(lock); } - lock->reader++; - qemu_co_mutex_unlock(&lock->mutex); - /* The rest of the read-side critical section is run without the mutex. */ self->locks_held++; } @@ -356,69 +404,64 @@ void qemu_co_rwlock_unlock(CoRwlock *lock) Coroutine *self = qemu_coroutine_self(); assert(qemu_in_coroutine()); - if (!lock->reader) { - /* The critical section started in qemu_co_rwlock_wrlock. */ - qemu_co_queue_restart_all(&lock->queue); - } else { - self->locks_held--; + self->locks_held--; - qemu_co_mutex_lock(&lock->mutex); - lock->reader--; - assert(lock->reader >= 0); - /* Wakeup only one waiting writer */ - if (!lock->reader) { - qemu_co_queue_next(&lock->queue); - } + qemu_co_mutex_lock(&lock->mutex); + if (lock->owners > 0) { + lock->owners--; + } else { + assert(lock->owners == -1); + lock->owners = 0; } - qemu_co_mutex_unlock(&lock->mutex); + + qemu_co_rwlock_maybe_wake_one(lock); } void qemu_co_rwlock_downgrade(CoRwlock *lock) { - Coroutine *self = qemu_coroutine_self(); - - /* lock->mutex critical section started in qemu_co_rwlock_wrlock or - * qemu_co_rwlock_upgrade. - */ - assert(lock->reader == 0); - lock->reader++; - qemu_co_mutex_unlock(&lock->mutex); + qemu_co_mutex_lock(&lock->mutex); + assert(lock->owners == -1); + lock->owners = 1; - /* The rest of the read-side critical section is run without the mutex. */ - self->locks_held++; + /* Possibly wake another reader, which will wake the next in line. */ + qemu_co_rwlock_maybe_wake_one(lock); } void qemu_co_rwlock_wrlock(CoRwlock *lock) { + Coroutine *self = qemu_coroutine_self(); + qemu_co_mutex_lock(&lock->mutex); - lock->pending_writer++; - while (lock->reader) { - qemu_co_queue_wait(&lock->queue, &lock->mutex); + if (lock->owners == 0) { + lock->owners = -1; + qemu_co_mutex_unlock(&lock->mutex); + } else { + CoRwTicket my_ticket = { false, qemu_coroutine_self() }; + + QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next); + qemu_co_mutex_unlock(&lock->mutex); + qemu_coroutine_yield(); + assert(lock->owners == -1); } - lock->pending_writer--; - /* The rest of the write-side critical section is run with - * the mutex taken, so that lock->reader remains zero. - * There is no need to update self->locks_held. - */ + self->locks_held++; } void qemu_co_rwlock_upgrade(CoRwlock *lock) { - Coroutine *self = qemu_coroutine_self(); - qemu_co_mutex_lock(&lock->mutex); - assert(lock->reader > 0); - lock->reader--; - lock->pending_writer++; - while (lock->reader) { - qemu_co_queue_wait(&lock->queue, &lock->mutex); - } - lock->pending_writer--; + assert(lock->owners > 0); + /* For fairness, wait if a writer is in line. */ + if (lock->owners == 1 && QSIMPLEQ_EMPTY(&lock->tickets)) { + lock->owners = -1; + qemu_co_mutex_unlock(&lock->mutex); + } else { + CoRwTicket my_ticket = { false, qemu_coroutine_self() }; - /* The rest of the write-side critical section is run with - * the mutex taken, similar to qemu_co_rwlock_wrlock. Do - * not account for the lock twice in self->locks_held. - */ - self->locks_held--; + lock->owners--; + QSIMPLEQ_INSERT_TAIL(&lock->tickets, &my_ticket, next); + qemu_co_rwlock_maybe_wake_one(lock); + qemu_coroutine_yield(); + assert(lock->owners == -1); + } } |