[PATCH v1 00/10] move memory module align to PostParse time

Daniel Henrique Barboza posted 10 patches 3 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201111220725.356297-1-danielhb413@gmail.com
Test syntax-check failed
NEWS.rst                                      |  14 +++
src/conf/domain_conf.c                        |  25 ++++-
src/conf/domain_conf.h                        |   2 +-
src/libvirt_private.syms                      |   2 +-
src/qemu/qemu_domain.c                        | 106 +++++++-----------
src/qemu/qemu_domain.h                        |   2 -
src/qemu/qemu_hotplug.c                       |   9 +-
tests/qemuxml2argvdata/hugepages-nvdimm.xml   |   2 +-
.../memory-hotplug-nvdimm-access.xml          |   2 +-
.../memory-hotplug-nvdimm-align.xml           |   2 +-
.../memory-hotplug-nvdimm-label.xml           |   2 +-
.../memory-hotplug-nvdimm-pmem.xml            |   2 +-
.../memory-hotplug-nvdimm-readonly.xml        |   2 +-
.../memory-hotplug-nvdimm.xml                 |   2 +-
.../memory-hotplug-ppc64-nonuma.args          |   2 +-
tests/qemuxml2argvdata/pages-dimm-discard.xml |   2 +-
.../memory-hotplug-dimm.xml                   |   4 +-
17 files changed, 92 insertions(+), 90 deletions(-)
[PATCH v1 00/10] move memory module align to PostParse time
Posted by Daniel Henrique Barboza 3 years, 5 months ago
Hi,

This is a work that I intended to do after pSeries NVDIMM changes
I made in commit v6.7.0-302-gd3f3c2c97f. The reasoning is that
if ppc64 NVDIMM alignment can be done in PostParse time, regular
ppc64 DIMMs alignment can also be done the same way. After that
I realized that the same logic can be applied to x86 as well,
and then I started to see where we could eliminate explicit
align calls.

The end result is that a bug that affects pSeries the most
was found and fixed in patch 02, and now we're able to reflect
the exact memory being used by the guest in the live XML since
the alignment is now done in parse time. The series can be
fetched from:

https://gitlab.com/danielhb/libvirt/-/tree/memory_modules_postparse_v1

NOTE: I worked with the x86 logic based on what I could assert
from the code itself, i.e. the aligment mechanics (2MiB align for
mem modules, 1MiB align for total memory) are related to QEMU
internals, not being hypervisor-agnostic. If someone with authority can
guarantee that the alignment restrictions for x86 are applicable
to all hypervisors (like in the pSeries case), let me know and
I'll respin the series putting the x86 code in
virDomainMemoryDefPostParse().


Daniel Henrique Barboza (10):
  domain_conf.c: handle all ppc64 dimms in
    virDomainNVDimmAlignSizePseries
  qemu_domain.c: align memory modules before calculating 'initialmem'
  domain_conf.c: align pSeries mem modules in
    virDomainMemoryDefPostParse()
  qemu_domain.c: simplify qemuDomainGetMemoryModuleSizeAlignment()
  qemu_domain.c: align x86 memory module in post parse time
  qemu_domain.c: move size check for mem modules after alignment
  qemu_hotplug.c: avoid aligning the mem obj in
    qemuDomainDetachPrepMemory()
  qemu_hotplug.c: avoid aligning the mem obj in qemuDomainAttachMemory()
  qemu_domain.c: remove qemuDomainMemoryDeviceAlignSize()
  NEWS.rs: document memory alignment improvements

 NEWS.rst                                      |  14 +++
 src/conf/domain_conf.c                        |  25 ++++-
 src/conf/domain_conf.h                        |   2 +-
 src/libvirt_private.syms                      |   2 +-
 src/qemu/qemu_domain.c                        | 106 +++++++-----------
 src/qemu/qemu_domain.h                        |   2 -
 src/qemu/qemu_hotplug.c                       |   9 +-
 tests/qemuxml2argvdata/hugepages-nvdimm.xml   |   2 +-
 .../memory-hotplug-nvdimm-access.xml          |   2 +-
 .../memory-hotplug-nvdimm-align.xml           |   2 +-
 .../memory-hotplug-nvdimm-label.xml           |   2 +-
 .../memory-hotplug-nvdimm-pmem.xml            |   2 +-
 .../memory-hotplug-nvdimm-readonly.xml        |   2 +-
 .../memory-hotplug-nvdimm.xml                 |   2 +-
 .../memory-hotplug-ppc64-nonuma.args          |   2 +-
 tests/qemuxml2argvdata/pages-dimm-discard.xml |   2 +-
 .../memory-hotplug-dimm.xml                   |   4 +-
 17 files changed, 92 insertions(+), 90 deletions(-)

-- 
2.26.2

Re: [PATCH v1 00/10] move memory module align to PostParse time
Posted by Michal Privoznik 3 years, 5 months ago
On 11/11/20 11:07 PM, Daniel Henrique Barboza wrote:
> Hi,
> 
> This is a work that I intended to do after pSeries NVDIMM changes
> I made in commit v6.7.0-302-gd3f3c2c97f. The reasoning is that
> if ppc64 NVDIMM alignment can be done in PostParse time, regular
> ppc64 DIMMs alignment can also be done the same way. After that
> I realized that the same logic can be applied to x86 as well,
> and then I started to see where we could eliminate explicit
> align calls.
> 
> The end result is that a bug that affects pSeries the most
> was found and fixed in patch 02, and now we're able to reflect
> the exact memory being used by the guest in the live XML since
> the alignment is now done in parse time. The series can be
> fetched from:
> 
> https://gitlab.com/danielhb/libvirt/-/tree/memory_modules_postparse_v1
> 
> NOTE: I worked with the x86 logic based on what I could assert
> from the code itself, i.e. the aligment mechanics (2MiB align for
> mem modules, 1MiB align for total memory) are related to QEMU
> internals, not being hypervisor-agnostic. If someone with authority can
> guarantee that the alignment restrictions for x86 are applicable
> to all hypervisors (like in the pSeries case), let me know and
> I'll respin the series putting the x86 code in
> virDomainMemoryDefPostParse().
> 
> 
> Daniel Henrique Barboza (10):
>    domain_conf.c: handle all ppc64 dimms in
>      virDomainNVDimmAlignSizePseries
>    qemu_domain.c: align memory modules before calculating 'initialmem'
>    domain_conf.c: align pSeries mem modules in
>      virDomainMemoryDefPostParse()
>    qemu_domain.c: simplify qemuDomainGetMemoryModuleSizeAlignment()
>    qemu_domain.c: align x86 memory module in post parse time
>    qemu_domain.c: move size check for mem modules after alignment
>    qemu_hotplug.c: avoid aligning the mem obj in
>      qemuDomainDetachPrepMemory()
>    qemu_hotplug.c: avoid aligning the mem obj in qemuDomainAttachMemory()
>    qemu_domain.c: remove qemuDomainMemoryDeviceAlignSize()
>    NEWS.rs: document memory alignment improvements
> 
>   NEWS.rst                                      |  14 +++
>   src/conf/domain_conf.c                        |  25 ++++-
>   src/conf/domain_conf.h                        |   2 +-
>   src/libvirt_private.syms                      |   2 +-
>   src/qemu/qemu_domain.c                        | 106 +++++++-----------
>   src/qemu/qemu_domain.h                        |   2 -
>   src/qemu/qemu_hotplug.c                       |   9 +-
>   tests/qemuxml2argvdata/hugepages-nvdimm.xml   |   2 +-
>   .../memory-hotplug-nvdimm-access.xml          |   2 +-
>   .../memory-hotplug-nvdimm-align.xml           |   2 +-
>   .../memory-hotplug-nvdimm-label.xml           |   2 +-
>   .../memory-hotplug-nvdimm-pmem.xml            |   2 +-
>   .../memory-hotplug-nvdimm-readonly.xml        |   2 +-
>   .../memory-hotplug-nvdimm.xml                 |   2 +-
>   .../memory-hotplug-ppc64-nonuma.args          |   2 +-
>   tests/qemuxml2argvdata/pages-dimm-discard.xml |   2 +-
>   .../memory-hotplug-dimm.xml                   |   4 +-
>   17 files changed, 92 insertions(+), 90 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

but perhaps you want Andrea to look into it too.

Michal

Re: [PATCH v1 00/10] move memory module align to PostParse time
Posted by Daniel Henrique Barboza 3 years, 5 months ago

On 11/12/20 8:28 AM, Michal Privoznik wrote:
> On 11/11/20 11:07 PM, Daniel Henrique Barboza wrote:
>> Hi,
>>
>> This is a work that I intended to do after pSeries NVDIMM changes
>> I made in commit v6.7.0-302-gd3f3c2c97f. The reasoning is that
>> if ppc64 NVDIMM alignment can be done in PostParse time, regular
>> ppc64 DIMMs alignment can also be done the same way. After that
>> I realized that the same logic can be applied to x86 as well,
>> and then I started to see where we could eliminate explicit
>> align calls.
>>
>> The end result is that a bug that affects pSeries the most
>> was found and fixed in patch 02, and now we're able to reflect
>> the exact memory being used by the guest in the live XML since
>> the alignment is now done in parse time. The series can be
>> fetched from:
>>
>> https://gitlab.com/danielhb/libvirt/-/tree/memory_modules_postparse_v1
>>
>> NOTE: I worked with the x86 logic based on what I could assert
>> from the code itself, i.e. the aligment mechanics (2MiB align for
>> mem modules, 1MiB align for total memory) are related to QEMU
>> internals, not being hypervisor-agnostic. If someone with authority can
>> guarantee that the alignment restrictions for x86 are applicable
>> to all hypervisors (like in the pSeries case), let me know and
>> I'll respin the series putting the x86 code in
>> virDomainMemoryDefPostParse().
>>
>>
>> Daniel Henrique Barboza (10):
>>    domain_conf.c: handle all ppc64 dimms in
>>      virDomainNVDimmAlignSizePseries
>>    qemu_domain.c: align memory modules before calculating 'initialmem'
>>    domain_conf.c: align pSeries mem modules in
>>      virDomainMemoryDefPostParse()
>>    qemu_domain.c: simplify qemuDomainGetMemoryModuleSizeAlignment()
>>    qemu_domain.c: align x86 memory module in post parse time
>>    qemu_domain.c: move size check for mem modules after alignment
>>    qemu_hotplug.c: avoid aligning the mem obj in
>>      qemuDomainDetachPrepMemory()
>>    qemu_hotplug.c: avoid aligning the mem obj in qemuDomainAttachMemory()
>>    qemu_domain.c: remove qemuDomainMemoryDeviceAlignSize()
>>    NEWS.rs: document memory alignment improvements
>>
>>   NEWS.rst                                      |  14 +++
>>   src/conf/domain_conf.c                        |  25 ++++-
>>   src/conf/domain_conf.h                        |   2 +-
>>   src/libvirt_private.syms                      |   2 +-
>>   src/qemu/qemu_domain.c                        | 106 +++++++-----------
>>   src/qemu/qemu_domain.h                        |   2 -
>>   src/qemu/qemu_hotplug.c                       |   9 +-
>>   tests/qemuxml2argvdata/hugepages-nvdimm.xml   |   2 +-
>>   .../memory-hotplug-nvdimm-access.xml          |   2 +-
>>   .../memory-hotplug-nvdimm-align.xml           |   2 +-
>>   .../memory-hotplug-nvdimm-label.xml           |   2 +-
>>   .../memory-hotplug-nvdimm-pmem.xml            |   2 +-
>>   .../memory-hotplug-nvdimm-readonly.xml        |   2 +-
>>   .../memory-hotplug-nvdimm.xml                 |   2 +-
>>   .../memory-hotplug-ppc64-nonuma.args          |   2 +-
>>   tests/qemuxml2argvdata/pages-dimm-discard.xml |   2 +-
>>   .../memory-hotplug-dimm.xml                   |   4 +-
>>   17 files changed, 92 insertions(+), 90 deletions(-)
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>


Thanks for the review!


> 
> but perhaps you want Andrea to look into it too.


Of course. Andrea, not sure if you want to review the whole series so here's
the gist: we already have the ppc64 NVDIMM alignment being done in PostParse time,
so I started to move all ppc64 memory module alignment to PostParse as well. In
the end I figured I could also move the alignment for x86 as well. Do you see any
potential problem with this design change?



Thanks,


DHB



> 
> Michal
> 

Re: [PATCH v1 00/10] move memory module align to PostParse time
Posted by Andrea Bolognani 3 years, 5 months ago
On Thu, 2020-11-12 at 18:53 -0300, Daniel Henrique Barboza wrote:
> On 11/12/20 8:28 AM, Michal Privoznik wrote:
> > On 11/11/20 11:07 PM, Daniel Henrique Barboza wrote:
> > >   NEWS.rst                                      |  14 +++
> > >   src/conf/domain_conf.c                        |  25 ++++-
> > >   src/conf/domain_conf.h                        |   2 +-
> > >   src/libvirt_private.syms                      |   2 +-
> > >   src/qemu/qemu_domain.c                        | 106 +++++++-----------
> > >   src/qemu/qemu_domain.h                        |   2 -
> > >   src/qemu/qemu_hotplug.c                       |   9 +-
> > >   tests/qemuxml2argvdata/hugepages-nvdimm.xml   |   2 +-
> > >   .../memory-hotplug-nvdimm-access.xml          |   2 +-
> > >   .../memory-hotplug-nvdimm-align.xml           |   2 +-
> > >   .../memory-hotplug-nvdimm-label.xml           |   2 +-
> > >   .../memory-hotplug-nvdimm-pmem.xml            |   2 +-
> > >   .../memory-hotplug-nvdimm-readonly.xml        |   2 +-
> > >   .../memory-hotplug-nvdimm.xml                 |   2 +-
> > >   .../memory-hotplug-ppc64-nonuma.args          |   2 +-
> > >   tests/qemuxml2argvdata/pages-dimm-discard.xml |   2 +-
> > >   .../memory-hotplug-dimm.xml                   |   4 +-
> > >   17 files changed, 92 insertions(+), 90 deletions(-)
> > 
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Thanks for the review!
> 
> > but perhaps you want Andrea to look into it too.
> 
> Of course. Andrea, not sure if you want to review the whole series so here's
> the gist: we already have the ppc64 NVDIMM alignment being done in PostParse time,
> so I started to move all ppc64 memory module alignment to PostParse as well. In
> the end I figured I could also move the alignment for x86 as well. Do you see any
> potential problem with this design change?

I'll take a look and let you know :)

-- 
Andrea Bolognani / Red Hat / Virtualization