[PATCH v3 0/8] memory: Sanity checks memory transaction when releasing BQL

Peter Xu posted 8 patches 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210728183151.195139-1-peterx@redhat.com
Maintainers: David Hildenbrand <david@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
cpus-common.c                  | 36 ++---------------------
include/exec/memory-internal.h |  1 +
include/hw/core/cpu.h          | 22 ++++++--------
softmmu/cpus.c                 | 39 ++++++++++++++++++++++---
softmmu/memory.c               | 53 ++++++++++++++++++++++++++++++----
5 files changed, 94 insertions(+), 57 deletions(-)
[PATCH v3 0/8] memory: Sanity checks memory transaction when releasing BQL
Posted by Peter Xu 2 years, 9 months ago
This is v3 of the series. And, I think this is 6.2 material.

v2 changelog:
- Init qemu_work_item directly in do_run_on_cpu() [David]
- Rename memory_region_transaction_{push|pop} to *_depth_{inc|dec} [David]
- Changing wording of s/helpless/useless/ [David]
- Squashed v2 patch 7 into patch 8 [David]
- Collected r-bs

It was actually got forgotten for months until it was used to identify another
potential issue of bql usage here (besides it could still be helpful when
debugging a previous kvm dirty ring issue in that series):

https://lore.kernel.org/qemu-devel/CH0PR02MB7898BBD73D0F3F7D5003BB178BE19@CH0PR02MB7898.namprd02.prod.outlook.com/

So I figured maybe it's still worth to have it, hence a repost.

There're some changes against v1:

  - patch "cpus: Introduce qemu_cond_timedwait_iothread()" is dropped because
    it's introduced in another commit already (b0c3cf9407e64).

  - two more patches to move do_run_on_cpu() into softmmu/ to fix a linux-user
    compliation issue.

Please review, thanks.

=== Original Cover letter ===

This is a continuous work of previous discussion on memory transactions [1].
It should be helpful to fail QEMU far earlier if there's misuse of BQL against
the QEMU memory model.

One example is run_on_cpu() during memory commit.  That'll work previously, but
it'll fail with very strange errors (like KVM ioctl failure due to memslot
already existed, and it's not guaranteed to trigger constantly).  Now it'll
directly fail when run_on_cpu() is called.

Please have a look, thanks.

[1] https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03205.html

Peter Xu (8):
  cpus: Export queue work related fields to cpu.h
  cpus: Move do_run_on_cpu into softmmu/cpus.c
  memory: Introduce memory_region_transaction_depth_{inc|dec}()
  memory: Don't do topology update in memory finalize()
  cpus: Use qemu_cond_wait_iothread() where proper
  cpus: Remove the mutex parameter from do_run_on_cpu()
  memory: Assert on no ongoing memory transaction before release BQL
  memory: Delay the transaction pop() until commit completed

 cpus-common.c                  | 36 ++---------------------
 include/exec/memory-internal.h |  1 +
 include/hw/core/cpu.h          | 22 ++++++--------
 softmmu/cpus.c                 | 39 ++++++++++++++++++++++---
 softmmu/memory.c               | 53 ++++++++++++++++++++++++++++++----
 5 files changed, 94 insertions(+), 57 deletions(-)

-- 
2.31.1



Re: [PATCH v3 0/8] memory: Sanity checks memory transaction when releasing BQL
Posted by Peter Xu 2 years, 7 months ago
On Wed, Jul 28, 2021 at 02:31:43PM -0400, Peter Xu wrote:
> This is v3 of the series. And, I think this is 6.2 material.

Ping - just remembered this one too..

-- 
Peter Xu