Message ID | 20200219160953.13771-1-imammedo@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | refactor main RAM allocation to use hostmem backend | expand |
Patchew URL: https://patchew.org/QEMU/20200219160953.13771-1-imammedo@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v6 00/79] refactor main RAM allocation to use hostmem backend Message-id: 20200219160953.13771-1-imammedo@redhat.com Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20200211160318.453650-1-stefanha@redhat.com -> patchew/20200211160318.453650-1-stefanha@redhat.com - [tag update] patchew/20200217150246.29180-1-vsementsov@virtuozzo.com -> patchew/20200217150246.29180-1-vsementsov@virtuozzo.com * [new tag] patchew/20200219160953.13771-1-imammedo@redhat.com -> patchew/20200219160953.13771-1-imammedo@redhat.com Switched to a new branch 'test' 9584b56 tests:numa-test: use explicit memdev to specify node RAM 786ed5c tests/numa-test: make top level args dynamic and g_autofree(cli) cleanups 4ebc74d hostmem: fix strict bind policy ffac16f hostmem: introduce "prealloc-threads" property 8b38de9 make mem_path local variable c001c3b exec: drop bogus mem_path from qemu_ram_alloc_from_fd() ad1172d exec: cleanup qemu_minrampagesize()/qemu_maxrampagesize() f0530f1 remove no longer used memory_region_allocate_system_memory() 769e8d9 sparc/niagara: use memdev for RAM b255475 sparc/sun4m: use memdev for RAM fe3e7b7 sparc/leon3: use memdev for RAM 9fe680e ppc/virtex_ml507: use memdev for RAM ab74e54 ppc/spapr: use memdev for RAM b28f018 ppc/{ppc440_bamboo, sam460ex}: use memdev for RAM a0258e4 ppc/{ppc440_bamboo, sam460ex}: drop RAM size fixup 2dc9ce1 ppc/ppc405_boards: use memdev for RAM 4428dcf ppc/ppc405_boards: add RAM size checks 173a36d ppc/pnv: use memdev for RAM 8ee06e4 ppc/mac_oldworld: use memdev for RAM a5b5de0 ppc/mac_newworld: use memdev for RAM 9731664 ppc/e500: use memdev for RAM 3538e84 ppc/e500: drop RAM size fixup ec88838 mips/mips_r4k: use memdev for RAM ceefaa3 mips/mips_mipssim: use memdev for RAM 3a6e6ac mips/mips_malta: use memdev for RAM 7c3dd4c mips/mips_jazz: add max ram size check 2a9bded mips/mips_jazz: use memdev for RAM 0de3d9f mips/mips_fulong2e: use memdev for RAM dc7b6ba mips/mips_fulong2e: drop RAM size fixup 9389d6c mips/boston: use memdev for RAM 49b64ba m68k/next-cube: use memdev for RAM 32c245c m68k/mcf5208: use memdev for RAM 8591a17 m68k/q800: use memdev for RAM c55f97a m68k/an5206: use memdev for RAM dc8953c lm32/milkymist: use memdev for RAM 6047c08 lm32/lm32_boards: use memdev for RAM bd45778 x86/pc: use memdev for RAM 9ad5468 x86/microvm: use memdev for RAM 7c59c1e hppa: use memdev for RAM 17c38c7 cris/axis_dev88: use memdev for RAM c74e719 null-machine: use memdev for RAM 3a12fc6 s390x/s390-virtio-ccw: use memdev for RAM 87c8047 arm/xlnx-zcu102: use memdev for RAM e920159 arm/xlnx-versal-virt: use memdev for RAM 8182d3d arm/xilinx_zynq: use memdev for RAM c980096 arm/xilinx_zynq: drop RAM size fixup a72f680 arm/virt: use memdev for RAM 08b8ba0 arm/vexpress: use memdev for RAM 6cf41f5 arm/versatilepb: use memdev for RAM 3818ed9 arm/sbsa-ref: use memdev for RAM a4317ae arm/raspi: use memdev for RAM 778f432 arm/sabrelite: use memdev for RAM 7f1679d arm/palm: use memdev for RAM 238ea0e arm/omap_sx1: use memdev for RAM 7998beb arm/nseries: use memdev for RAM 3ed6131 arm/musicpal: use memdev for RAM 68637c3 arm/mps2: use memdev for RAM 70a2cb8 arm/mps2-tz: use memdev for RAM 4076cc9 arm/mcimx7d-sabre: use memdev for RAM 14dbfa5 arm/mcimx6ul-evk: use memdev for RAM 3865cfa arm/kzm: use memdev for RAM 462f1f4 arm/kzm: drop RAM size fixup 3f25b3f arm/integratorcp: use memdev for RAM eebd06a arm/imx25_pdk: use memdev for RAM bf350da arm/imx25_pdk: drop RAM size fixup 89c43bd arm/highbank: use memdev for RAM 4daf95d arm/digic_boards: use memdev for RAM 0f07fe3 arm/cubieboard: use memdev for RAM 00b9829 arm/collie: use memdev for RAM afcbaed arm/aspeed: use memdev for RAM 533eb41 arm/aspeed: actually check RAM size b844d82 alpha/dp264: use memdev for RAM fe64d06 vl.c: ensure that ram_size matches size of machine.memory-backend a1b18df vl.c: move -m parsing after memory backends has been processed 6b61c2c initialize MachineState::ram in NUMA case 82b911a machine: introduce convenience MachineState::ram 900c0ba machine: alias -mem-path and -mem-prealloc into memory-foo backend aa8b183 machine: introduce memory-backend property 68a86dc numa: remove deprecated -mem-path fallback to anonymous RAM === OUTPUT BEGIN === 1/79 Checking commit 68a86dc15ccd (numa: remove deprecated -mem-path fallback to anonymous RAM) 2/79 Checking commit aa8b183974b2 (machine: introduce memory-backend property) 3/79 Checking commit 900c0ba373aa (machine: alias -mem-path and -mem-prealloc into memory-foo backend) 4/79 Checking commit 82b911aaff3b (machine: introduce convenience MachineState::ram) 5/79 Checking commit 6b61c2c596e7 (initialize MachineState::ram in NUMA case) 6/79 Checking commit a1b18df9a484 (vl.c: move -m parsing after memory backends has been processed) 7/79 Checking commit fe64d06afc1c (vl.c: ensure that ram_size matches size of machine.memory-backend) 8/79 Checking commit b844d822cf62 (alpha/dp264: use memdev for RAM) ERROR: spaces required around that '*' (ctx:WxV) #30: FILE: hw/alpha/alpha_sys.h:14: +PCIBus *typhoon_init(MemoryRegion *, ISABus **, qemu_irq *, AlphaCPU *[4], ^ total: 1 errors, 0 warnings, 49 lines checked Patch 8/79 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 9/79 Checking commit 533eb415df2e (arm/aspeed: actually check RAM size) 10/79 Checking commit afcbaed668fc (arm/aspeed: use memdev for RAM) 11/79 Checking commit 00b9829f83c0 (arm/collie: use memdev for RAM) 12/79 Checking commit 0f07fe38e4a1 (arm/cubieboard: use memdev for RAM) 13/79 Checking commit 4daf95d60791 (arm/digic_boards: use memdev for RAM) 14/79 Checking commit 89c43bdf20ad (arm/highbank: use memdev for RAM) 15/79 Checking commit bf350daae024 (arm/imx25_pdk: drop RAM size fixup) 16/79 Checking commit eebd06abc6ac (arm/imx25_pdk: use memdev for RAM) 17/79 Checking commit 3f25b3f4e88f (arm/integratorcp: use memdev for RAM) 18/79 Checking commit 462f1f4bde2a (arm/kzm: drop RAM size fixup) 19/79 Checking commit 3865cfacfe47 (arm/kzm: use memdev for RAM) 20/79 Checking commit 14dbfa556ba2 (arm/mcimx6ul-evk: use memdev for RAM) 21/79 Checking commit 4076cc942932 (arm/mcimx7d-sabre: use memdev for RAM) 22/79 Checking commit 70a2cb8e8d0f (arm/mps2-tz: use memdev for RAM) 23/79 Checking commit 68637c3a3687 (arm/mps2: use memdev for RAM) 24/79 Checking commit 3ed61312bdb8 (arm/musicpal: use memdev for RAM) 25/79 Checking commit 7998beb9c2e2 (arm/nseries: use memdev for RAM) 26/79 Checking commit 238ea0e31156 (arm/omap_sx1: use memdev for RAM) 27/79 Checking commit 7f1679dc2cee (arm/palm: use memdev for RAM) 28/79 Checking commit 778f43267a3f (arm/sabrelite: use memdev for RAM) 29/79 Checking commit a4317ae8ba5e (arm/raspi: use memdev for RAM) 30/79 Checking commit 3818ed92dc5d (arm/sbsa-ref: use memdev for RAM) 31/79 Checking commit 6cf41f5586c4 (arm/versatilepb: use memdev for RAM) 32/79 Checking commit 08b8ba04c9d0 (arm/vexpress: use memdev for RAM) 33/79 Checking commit a72f6805f30c (arm/virt: use memdev for RAM) 34/79 Checking commit c9800965c1be (arm/xilinx_zynq: drop RAM size fixup) 35/79 Checking commit 8182d3d1f186 (arm/xilinx_zynq: use memdev for RAM) 36/79 Checking commit e9201598f428 (arm/xlnx-versal-virt: use memdev for RAM) 37/79 Checking commit 87c8047f65b4 (arm/xlnx-zcu102: use memdev for RAM) 38/79 Checking commit 3a12fc61af5c (s390x/s390-virtio-ccw: use memdev for RAM) 39/79 Checking commit c74e71908d28 (null-machine: use memdev for RAM) 40/79 Checking commit 17c38c759c05 (cris/axis_dev88: use memdev for RAM) 41/79 Checking commit 7c59c1e0cced (hppa: use memdev for RAM) 42/79 Checking commit 9ad54686924b (x86/microvm: use memdev for RAM) 43/79 Checking commit bd457782b3b0 (x86/pc: use memdev for RAM) 44/79 Checking commit 6047c08fd714 (lm32/lm32_boards: use memdev for RAM) 45/79 Checking commit dc8953c6be14 (lm32/milkymist: use memdev for RAM) 46/79 Checking commit c55f97a0e157 (m68k/an5206: use memdev for RAM) 47/79 Checking commit 8591a179af02 (m68k/q800: use memdev for RAM) 48/79 Checking commit 32c245cfaf27 (m68k/mcf5208: use memdev for RAM) 49/79 Checking commit 49b64ba90649 (m68k/next-cube: use memdev for RAM) 50/79 Checking commit 9389d6ce0b89 (mips/boston: use memdev for RAM) 51/79 Checking commit dc7b6ba5b20c (mips/mips_fulong2e: drop RAM size fixup) 52/79 Checking commit 0de3d9fba60f (mips/mips_fulong2e: use memdev for RAM) 53/79 Checking commit 2a9bded9a337 (mips/mips_jazz: use memdev for RAM) 54/79 Checking commit 7c3dd4c6a50e (mips/mips_jazz: add max ram size check) 55/79 Checking commit 3a6e6ac76293 (mips/mips_malta: use memdev for RAM) 56/79 Checking commit ceefaa3b245f (mips/mips_mipssim: use memdev for RAM) 57/79 Checking commit ec88838cdc77 (mips/mips_r4k: use memdev for RAM) 58/79 Checking commit 3538e846cb8d (ppc/e500: drop RAM size fixup) 59/79 Checking commit 9731664559a3 (ppc/e500: use memdev for RAM) 60/79 Checking commit a5b5de02ac5b (ppc/mac_newworld: use memdev for RAM) 61/79 Checking commit 8ee06e4ccb0f (ppc/mac_oldworld: use memdev for RAM) 62/79 Checking commit 173a36d8d165 (ppc/pnv: use memdev for RAM) 63/79 Checking commit 4428dcf7b990 (ppc/ppc405_boards: add RAM size checks) 64/79 Checking commit 2dc9ce13d210 (ppc/ppc405_boards: use memdev for RAM) 65/79 Checking commit a0258e4afa10 (ppc/{ppc440_bamboo, sam460ex}: drop RAM size fixup) 66/79 Checking commit b28f01880eda (ppc/{ppc440_bamboo, sam460ex}: use memdev for RAM) 67/79 Checking commit ab74e5431129 (ppc/spapr: use memdev for RAM) 68/79 Checking commit 9fe680ee7578 (ppc/virtex_ml507: use memdev for RAM) 69/79 Checking commit fe3e7b71e68a (sparc/leon3: use memdev for RAM) 70/79 Checking commit b2554752b1da (sparc/sun4m: use memdev for RAM) 71/79 Checking commit 769e8d93bebf (sparc/niagara: use memdev for RAM) 72/79 Checking commit f0530f14c7c3 (remove no longer used memory_region_allocate_system_memory()) 73/79 Checking commit ad1172d8e56e (exec: cleanup qemu_minrampagesize()/qemu_maxrampagesize()) 74/79 Checking commit c001c3b3d93a (exec: drop bogus mem_path from qemu_ram_alloc_from_fd()) 75/79 Checking commit 8b38de9f62cf (make mem_path local variable) 76/79 Checking commit ffac16fab33b (hostmem: introduce "prealloc-threads" property) 77/79 Checking commit 4ebc74dbbf7a (hostmem: fix strict bind policy) 78/79 Checking commit 786ed5c497fd (tests/numa-test: make top level args dynamic and g_autofree(cli) cleanups) 79/79 Checking commit 9584b5641981 (tests:numa-test: use explicit memdev to specify node RAM) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200219160953.13771-1-imammedo@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Hi Igor, On 2/19/20 5:08 PM, Igor Mammedov wrote: [...] > Series removes ad hoc RAM allocation API (memory_region_allocate_system_memory) > and consolidates it around hostmem backend. It allows to > * resolve conflicts between global -mem-prealloc and hostmem's "policy" option > fixing premature allocation before binding policy is applied > * simplify complicated memory allocation routines which had to deal with 2 ways > to allocate RAM. > * it allows to reuse hostmem backends of a choice for main RAM without adding > extra CLI options to duplicate hostmem features. > Recent case was -mem-shared, to enable vhost-user on targets that don't > support hostmem backends [1] (ex: s390) > * move RAM allocation from individual boards into generic machine code and > provide them with prepared MemoryRegion. > * clean up deprecated NUMA features which were tied to the old API (see patches) > - "numa: remove deprecated -mem-path fallback to anonymous RAM" > - (POSTPONED, waiting on libvirt side) "forbid '-numa node,mem' for 5.0 and newer machine types" > - (POSTPONED) "numa: remove deprecated implicit RAM distribution between nodes" > > Conversion introduces a new machine.memory-backend property and wrapper code that > aliases global -mem-path and -mem-alloc into automatically created hostmem > backend properties (provided memory-backend was not set explicitly given by user). > And then follows bulk of trivial patches that incrementally convert individual > boards to using machine.memory-backend provided MemoryRegion. > > Board conversion typically involves: > * providing MachineClass::default_ram_size and MachineClass::default_ram_id > so generic code could create default backend if user didn't explicitly provide > memory-backend or -m options > * dropping memory_region_allocate_system_memory() call > * using convenience MachineState::ram MemoryRegion, which points to MemoryRegion > allocated by ram-memdev > On top of that for some boards: > * added missing ram_size checks (typically it were boards with fixed ram size) > * ram_size fixups were replaced by checks and hard errors, forcing user to > provide correct "-m" values instead of ignoring it and continuing running. > > After all boards are converted the old API is removed and memory allocation > routines are cleaned up. I wonder about the pre-QOM machines. As they don't call memory_region_allocate_system_memory(), the conversion is not required? (See for example pxa270_init).
On Mon, 24 Feb 2020 09:45:11 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Igor, > > On 2/19/20 5:08 PM, Igor Mammedov wrote: > [...] > > Series removes ad hoc RAM allocation API (memory_region_allocate_system_memory) > > and consolidates it around hostmem backend. It allows to > > * resolve conflicts between global -mem-prealloc and hostmem's "policy" option > > fixing premature allocation before binding policy is applied > > * simplify complicated memory allocation routines which had to deal with 2 ways > > to allocate RAM. > > * it allows to reuse hostmem backends of a choice for main RAM without adding > > extra CLI options to duplicate hostmem features. > > Recent case was -mem-shared, to enable vhost-user on targets that don't > > support hostmem backends [1] (ex: s390) > > * move RAM allocation from individual boards into generic machine code and > > provide them with prepared MemoryRegion. > > * clean up deprecated NUMA features which were tied to the old API (see patches) > > - "numa: remove deprecated -mem-path fallback to anonymous RAM" > > - (POSTPONED, waiting on libvirt side) "forbid '-numa node,mem' for 5.0 and newer machine types" > > - (POSTPONED) "numa: remove deprecated implicit RAM distribution between nodes" > > > > Conversion introduces a new machine.memory-backend property and wrapper code that > > aliases global -mem-path and -mem-alloc into automatically created hostmem > > backend properties (provided memory-backend was not set explicitly given by user). > > And then follows bulk of trivial patches that incrementally convert individual > > boards to using machine.memory-backend provided MemoryRegion. > > > > Board conversion typically involves: > > * providing MachineClass::default_ram_size and MachineClass::default_ram_id > > so generic code could create default backend if user didn't explicitly provide > > memory-backend or -m options > > * dropping memory_region_allocate_system_memory() call > > * using convenience MachineState::ram MemoryRegion, which points to MemoryRegion > > allocated by ram-memdev > > On top of that for some boards: > > * added missing ram_size checks (typically it were boards with fixed ram size) > > * ram_size fixups were replaced by checks and hard errors, forcing user to > > provide correct "-m" values instead of ignoring it and continuing running. > > > > After all boards are converted the old API is removed and memory allocation > > routines are cleaned up. > > I wonder about the pre-QOM machines. As they don't call > memory_region_allocate_system_memory(), the conversion is not required? > (See for example pxa270_init). Since they weren't using memory_region_allocate_system_memory(), they are out of scope of this series. As for the future, I'd only make boards that support user configurable ram size to accept "-m". For fixed size boards -m/memdev is overkill and we need to decide what to do with them. I see following options (in order of my preference): 1. Non popular: error out if -m is specified (it used to work, but not anymore when check is added, i.e similar to size checks introduced in this series so users have to adapt their CLI). It can still use automatically created memdev but I'd ditch it on those boards and use plain memory_region_init_ram(). This is matches well SoCs that have embedded RAM and don't really care about what user may specify with -m. It would simplify simple boards. 2. a path of least resistance: continue support -m and generalize ram_size checks for such boards. This could use memdev since it comes for free with -m support. I don't expect complications with generalizing it (but one would only know for sure when it's coded) The next this I plan to do is to clean up ram_size global and hopefully get rid of MachineState:ram_size as well.
On 2/24/20 12:33 PM, Igor Mammedov wrote: > On Mon, 24 Feb 2020 09:45:11 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Igor, >> >> On 2/19/20 5:08 PM, Igor Mammedov wrote: >> [...] >>> Series removes ad hoc RAM allocation API (memory_region_allocate_system_memory) >>> and consolidates it around hostmem backend. It allows to >>> * resolve conflicts between global -mem-prealloc and hostmem's "policy" option >>> fixing premature allocation before binding policy is applied >>> * simplify complicated memory allocation routines which had to deal with 2 ways >>> to allocate RAM. >>> * it allows to reuse hostmem backends of a choice for main RAM without adding >>> extra CLI options to duplicate hostmem features. >>> Recent case was -mem-shared, to enable vhost-user on targets that don't >>> support hostmem backends [1] (ex: s390) >>> * move RAM allocation from individual boards into generic machine code and >>> provide them with prepared MemoryRegion. >>> * clean up deprecated NUMA features which were tied to the old API (see patches) >>> - "numa: remove deprecated -mem-path fallback to anonymous RAM" >>> - (POSTPONED, waiting on libvirt side) "forbid '-numa node,mem' for 5.0 and newer machine types" >>> - (POSTPONED) "numa: remove deprecated implicit RAM distribution between nodes" >>> >>> Conversion introduces a new machine.memory-backend property and wrapper code that >>> aliases global -mem-path and -mem-alloc into automatically created hostmem >>> backend properties (provided memory-backend was not set explicitly given by user). >>> And then follows bulk of trivial patches that incrementally convert individual >>> boards to using machine.memory-backend provided MemoryRegion. >>> >>> Board conversion typically involves: >>> * providing MachineClass::default_ram_size and MachineClass::default_ram_id >>> so generic code could create default backend if user didn't explicitly provide >>> memory-backend or -m options >>> * dropping memory_region_allocate_system_memory() call >>> * using convenience MachineState::ram MemoryRegion, which points to MemoryRegion >>> allocated by ram-memdev >>> On top of that for some boards: >>> * added missing ram_size checks (typically it were boards with fixed ram size) >>> * ram_size fixups were replaced by checks and hard errors, forcing user to >>> provide correct "-m" values instead of ignoring it and continuing running. >>> >>> After all boards are converted the old API is removed and memory allocation >>> routines are cleaned up. >> >> I wonder about the pre-QOM machines. As they don't call >> memory_region_allocate_system_memory(), the conversion is not required? >> (See for example pxa270_init). > Since they weren't using memory_region_allocate_system_memory(), they are > out of scope of this series. > > As for the future, I'd only make boards that support user configurable > ram size to accept "-m". Good cleanup. > > For fixed size boards -m/memdev is overkill and we need to decide what to do > with them. I see following options (in order of my preference): > 1. Non popular: error out if -m is specified (it used to work, but not > anymore when check is added, i.e similar to size checks > introduced in this series so users have to adapt their CLI). > It can still use automatically created memdev but I'd ditch it on > those boards and use plain memory_region_init_ram(). > This is matches well SoCs that have embedded RAM and don't really > care about what user may specify with -m. It would simplify > simple boards. LGTM. > > 2. a path of least resistance: continue support -m and generalize > ram_size checks for such boards. This could use memdev since it > comes for free with -m support. I don't expect complications > with generalizing it (but one would only know for sure when > it's coded) > > The next this I plan to do is to clean up ram_size global and > hopefully get rid of MachineState:ram_size as well. >