| Message ID | 20170907092010.3605-2-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
| Series | memory: Reduce memory use | expand |
On 7 September 2017 at 10:20, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > Most devices use at least one address space and every time a new address > space is added, flat views and dispatch trees are rebuild for all address > spaces. This is not a problem for a relatively small amount of devices but > even 50 virtio-pci devices use more than 8GB of RAM. > > What happens that on every flatview/dispatch rebuild, new arrays are > allocated and old ones release but the release is done via RCU so until > an entire machine is build, they are not released. > > This wraps devices creation into memory_region_transaction_begin/commit > to massively reduce amount of flat view/dispatch tree (re)allocations. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > vl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/vl.c b/vl.c > index 8e247cc2a2..3c39cc8b3a 100644 > --- a/vl.c > +++ b/vl.c > @@ -4655,12 +4655,16 @@ int main(int argc, char **argv, char **envp) > igd_gfx_passthru(); > > /* init generic devices */ > + memory_region_transaction_begin(); > + > rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); > if (qemu_opts_foreach(qemu_find_opts("device"), > device_init_func, NULL, NULL)) { > exit(1); > } > > + memory_region_transaction_commit(); What happens if something in a device realize function tries to do a read from an AddressSpace? I can't think of any examples that need to do that and I suspect it's probably a bug if anybody tries it, but I'm curious whether this change alters the failure mode... thanks -- PMM
On 07/09/17 19:30, Peter Maydell wrote: > On 7 September 2017 at 10:20, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >> Most devices use at least one address space and every time a new address >> space is added, flat views and dispatch trees are rebuild for all address >> spaces. This is not a problem for a relatively small amount of devices but >> even 50 virtio-pci devices use more than 8GB of RAM. >> >> What happens that on every flatview/dispatch rebuild, new arrays are >> allocated and old ones release but the release is done via RCU so until >> an entire machine is build, they are not released. >> >> This wraps devices creation into memory_region_transaction_begin/commit >> to massively reduce amount of flat view/dispatch tree (re)allocations. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> vl.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/vl.c b/vl.c >> index 8e247cc2a2..3c39cc8b3a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -4655,12 +4655,16 @@ int main(int argc, char **argv, char **envp) >> igd_gfx_passthru(); >> >> /* init generic devices */ >> + memory_region_transaction_begin(); >> + >> rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); >> if (qemu_opts_foreach(qemu_find_opts("device"), >> device_init_func, NULL, NULL)) { >> exit(1); >> } >> >> + memory_region_transaction_commit(); > > What happens if something in a device realize function tries > to do a read from an AddressSpace? address_space_memory is created before that loop but yes, address spaces for devices are not rendered and QEMU crashes, needs fixing. > I can't think of any examples > that need to do that and I suspect it's probably a bug if anybody > tries it, but I'm curious whether this change alters the failure > mode...
On 7 September 2017 at 15:27, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > On 07/09/17 19:30, Peter Maydell wrote: >> What happens if something in a device realize function tries >> to do a read from an AddressSpace? > > > address_space_memory is created before that loop but yes, address spaces > for devices are not rendered and QEMU crashes, needs fixing. I'd be OK if we defined this to be not permitted, as long as we're reasonably happy that we don't currently do it anywhere and the error if it does happen is fairly clear about what's gone wrong. thanks -- PMM
On 08/09/17 00:30, Peter Maydell wrote: > On 7 September 2017 at 15:27, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: >> On 07/09/17 19:30, Peter Maydell wrote: >>> What happens if something in a device realize function tries >>> to do a read from an AddressSpace? >> >> >> address_space_memory is created before that loop but yes, address spaces >> for devices are not rendered and QEMU crashes, needs fixing. > > I'd be OK if we defined this to be not permitted, as long as > we're reasonably happy that we don't currently do it anywhere > and the error if it does happen is fairly clear about what's > gone wrong. QEMU actually only crashes with 2/4 + 3/4, with just 1/4 QEMU gets to unassigned_mem_accepts() silently. Not sure how to convert this to a clear error message.
diff --git a/vl.c b/vl.c index 8e247cc2a2..3c39cc8b3a 100644 --- a/vl.c +++ b/vl.c @@ -4655,12 +4655,16 @@ int main(int argc, char **argv, char **envp) igd_gfx_passthru(); /* init generic devices */ + memory_region_transaction_begin(); + rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE); if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, NULL)) { exit(1); } + memory_region_transaction_commit(); + cpu_synchronize_all_post_init(); rom_reset_order_override();
Most devices use at least one address space and every time a new address space is added, flat views and dispatch trees are rebuild for all address spaces. This is not a problem for a relatively small amount of devices but even 50 virtio-pci devices use more than 8GB of RAM. What happens that on every flatview/dispatch rebuild, new arrays are allocated and old ones release but the release is done via RCU so until an entire machine is build, they are not released. This wraps devices creation into memory_region_transaction_begin/commit to massively reduce amount of flat view/dispatch tree (re)allocations. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- vl.c | 4 ++++ 1 file changed, 4 insertions(+)