Message ID | 20200806150726.962-1-philmd@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PATCH-for-5.2] exec: Remove MemoryRegion::global_locking field | expand |
On Thu, Aug 06, 2020 at 05:07:26PM +0200, Philippe Mathieu-Daudé wrote: > Last uses of memory_region_clear_global_locking() have been > removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked") > and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O > path to TLB path"). > Remove memory_region_clear_global_locking() and the now unused > 'global_locking' field in MemoryRegion. > > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Suggested-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/exec/memory.h | 14 -------------- > accel/tcg/cputlb.c | 4 ++-- > exec.c | 2 +- > softmmu/memory.c | 6 ------ > 4 files changed, 3 insertions(+), 23 deletions(-) It can be added back in later, if necessary. For now let's drop the dead code. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 07/08/20 12:02, Stefan Hajnoczi wrote: > On Thu, Aug 06, 2020 at 05:07:26PM +0200, Philippe Mathieu-Daudé wrote: >> Last uses of memory_region_clear_global_locking() have been >> removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked") >> and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O >> path to TLB path"). >> Remove memory_region_clear_global_locking() and the now unused >> 'global_locking' field in MemoryRegion. >> >> Reported-by: Alexander Bulekov <alxndr@bu.edu> >> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/exec/memory.h | 14 -------------- >> accel/tcg/cputlb.c | 4 ++-- >> exec.c | 2 +- >> softmmu/memory.c | 6 ------ >> 4 files changed, 3 insertions(+), 23 deletions(-) > > It can be added back in later, if necessary. For now let's drop the dead > code. > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > I expect it will come back since Linaro is working on BQL-free interrupt handling for TCG, but no objections. Paolo
+Robert On 8/7/20 12:16 PM, Paolo Bonzini wrote: > On 07/08/20 12:02, Stefan Hajnoczi wrote: >> On Thu, Aug 06, 2020 at 05:07:26PM +0200, Philippe Mathieu-Daudé wrote: >>> Last uses of memory_region_clear_global_locking() have been >>> removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked") >>> and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O >>> path to TLB path"). >>> Remove memory_region_clear_global_locking() and the now unused >>> 'global_locking' field in MemoryRegion. >>> >>> Reported-by: Alexander Bulekov <alxndr@bu.edu> >>> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> include/exec/memory.h | 14 -------------- >>> accel/tcg/cputlb.c | 4 ++-- >>> exec.c | 2 +- >>> softmmu/memory.c | 6 ------ >>> 4 files changed, 3 insertions(+), 23 deletions(-) >> >> It can be added back in later, if necessary. For now let's drop the dead >> code. >> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> > > I expect it will come back since Linaro is working on BQL-free interrupt > handling for TCG, but no objections. Robert, any comment on this patch? Thanks, Phil.
On Sat, 22 Aug 2020 at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > +Robert > > On 8/7/20 12:16 PM, Paolo Bonzini wrote: > > On 07/08/20 12:02, Stefan Hajnoczi wrote: > >> On Thu, Aug 06, 2020 at 05:07:26PM +0200, Philippe Mathieu-Daudé wrote: > >>> Last uses of memory_region_clear_global_locking() have been > >>> removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked") > >>> and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O > >>> path to TLB path"). > >>> Remove memory_region_clear_global_locking() and the now unused > >>> 'global_locking' field in MemoryRegion. > >>> > >>> Reported-by: Alexander Bulekov <alxndr@bu.edu> > >>> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com> > >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >>> --- > >>> include/exec/memory.h | 14 -------------- > >>> accel/tcg/cputlb.c | 4 ++-- > >>> exec.c | 2 +- > >>> softmmu/memory.c | 6 ------ > >>> 4 files changed, 3 insertions(+), 23 deletions(-) > >> > >> It can be added back in later, if necessary. For now let's drop the dead > >> code. > >> > >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > >> > > > > I expect it will come back since Linaro is working on BQL-free interrupt > > handling for TCG, but no objections. > > Robert, any comment on this patch? > > Thanks, > > Phil. > Phil, Thanks for the heads up on this ! No objections to removing this. We can easily re-add it if/when needed. Thanks, -Rob
diff --git a/include/exec/memory.h b/include/exec/memory.h index 307e527835..bb84283376 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -377,7 +377,6 @@ struct MemoryRegion { bool nonvolatile; bool rom_device; bool flush_coalesced_mmio; - bool global_locking; uint8_t dirty_log_mask; bool is_iommu; RAMBlock *ram_block; @@ -1711,19 +1710,6 @@ void memory_region_set_flush_coalesced(MemoryRegion *mr); */ void memory_region_clear_flush_coalesced(MemoryRegion *mr); -/** - * memory_region_clear_global_locking: Declares that access processing does - * not depend on the QEMU global lock. - * - * By clearing this property, accesses to the memory region will be processed - * outside of QEMU's global lock (unless the lock is held on when issuing the - * access request). In this case, the device model implementing the access - * handlers is responsible for synchronization of concurrency. - * - * @mr: the memory region to be updated. - */ -void memory_region_clear_global_locking(MemoryRegion *mr); - /** * memory_region_add_eventfd: Request an eventfd to be triggered when a word * is written to a location. diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index 5698292749..9ddf2b8dfa 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1053,7 +1053,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, cpu_io_recompile(cpu, retaddr); } - if (mr->global_locking && !qemu_mutex_iothread_locked()) { + if (!qemu_mutex_iothread_locked()) { qemu_mutex_lock_iothread(); locked = true; } @@ -1114,7 +1114,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, */ save_iotlb_data(cpu, iotlbentry->addr, section, mr_offset); - if (mr->global_locking && !qemu_mutex_iothread_locked()) { + if (!qemu_mutex_iothread_locked()) { qemu_mutex_lock_iothread(); locked = true; } diff --git a/exec.c b/exec.c index 6f381f98e2..05cfc34b6c 100644 --- a/exec.c +++ b/exec.c @@ -3135,7 +3135,7 @@ static bool prepare_mmio_access(MemoryRegion *mr) bool unlocked = !qemu_mutex_iothread_locked(); bool release_lock = false; - if (unlocked && mr->global_locking) { + if (unlocked) { qemu_mutex_lock_iothread(); unlocked = false; release_lock = true; diff --git a/softmmu/memory.c b/softmmu/memory.c index af25987518..2303747feb 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1221,7 +1221,6 @@ static void memory_region_initfn(Object *obj) mr->ops = &unassigned_mem_ops; mr->enabled = true; mr->romd_mode = true; - mr->global_locking = true; mr->destructor = memory_region_destructor_none; QTAILQ_INIT(&mr->subregions); QTAILQ_INIT(&mr->coalesced); @@ -2277,11 +2276,6 @@ void memory_region_clear_flush_coalesced(MemoryRegion *mr) } } -void memory_region_clear_global_locking(MemoryRegion *mr) -{ - mr->global_locking = false; -} - static bool userspace_eventfd_warning; void memory_region_add_eventfd(MemoryRegion *mr,
Last uses of memory_region_clear_global_locking() have been removed in commit 7070e085d4 ("acpi: mark PMTIMER as unlocked") and commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O path to TLB path"). Remove memory_region_clear_global_locking() and the now unused 'global_locking' field in MemoryRegion. Reported-by: Alexander Bulekov <alxndr@bu.edu> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/exec/memory.h | 14 -------------- accel/tcg/cputlb.c | 4 ++-- exec.c | 2 +- softmmu/memory.c | 6 ------ 4 files changed, 3 insertions(+), 23 deletions(-)