mbox series

[0/3] Restore extra_mem_pages and add slot0_mem_pages

Message ID 20210608233816.423958-1-zhenzhong.duan@intel.com (mailing list archive)
Headers show
Series Restore extra_mem_pages and add slot0_mem_pages | expand

Message

Duan, Zhenzhong June 8, 2021, 11:38 p.m. UTC
(39fe2fc96694 "selftests: kvm: make allocation of extra memory take effect")
changed the meaning of extra_mem_pages and treated it as slot0 memory size.

In fact extra_mem_pages is used for non-slot0 memory size, there is no custom
slot0 memory size support. See discuss in https://lkml.org/lkml/2021/6/3/551
for more details.

This patchset restores extra_mem_pages's original meaning and adds support for
custom slot0 memory with a new parameter slot0_mem_pages.

Run below command, all 39 tests passed.
# make -C tools/testing/selftests/ TARGETS=kvm run_tests

Zhenzhong Duan (3):
  Revert "selftests: kvm: make allocation of extra memory take effect"
  Revert "selftests: kvm: fix overlapping addresses in
    memslot_perf_test"
  selftests: kvm: Add support for customized slot0 memory size

 .../testing/selftests/kvm/include/kvm_util.h  |  7 +--
 .../selftests/kvm/kvm_page_table_test.c       |  2 +-
 tools/testing/selftests/kvm/lib/kvm_util.c    | 47 +++++++++++++++----
 .../selftests/kvm/lib/perf_test_util.c        |  2 +-
 .../testing/selftests/kvm/memslot_perf_test.c |  2 +-
 5 files changed, 45 insertions(+), 15 deletions(-)

Comments

Maciej S. Szmigiero June 8, 2021, 4:46 p.m. UTC | #1
On 09.06.2021 01:38, Zhenzhong Duan wrote:
> (39fe2fc96694 "selftests: kvm: make allocation of extra memory take effect")
> changed the meaning of extra_mem_pages and treated it as slot0 memory size.
> 
> In fact extra_mem_pages is used for non-slot0 memory size, there is no custom
> slot0 memory size support. See discuss in https://urldefense.com/v3/__https://lkml.org/lkml/2021/6/3/551__;!!GqivPVa7Brio!K2FcwkE2nzlPAWgBHh6o6jtaWe66RMlfkb-b_8mAtVa5d8ez_sArupY-EqIquuCj2sorww$
> for more details.
> 
> This patchset restores extra_mem_pages's original meaning and adds support for
> custom slot0 memory with a new parameter slot0_mem_pages.
> 
> Run below command, all 39 tests passed.
> # make -C tools/testing/selftests/ TARGETS=kvm run_tests
> 
> Zhenzhong Duan (3):
>    Revert "selftests: kvm: make allocation of extra memory take effect"
>    Revert "selftests: kvm: fix overlapping addresses in
>      memslot_perf_test"
>    selftests: kvm: Add support for customized slot0 memory size
> 
>   .../testing/selftests/kvm/include/kvm_util.h  |  7 +--
>   .../selftests/kvm/kvm_page_table_test.c       |  2 +-
>   tools/testing/selftests/kvm/lib/kvm_util.c    | 47 +++++++++++++++----
>   .../selftests/kvm/lib/perf_test_util.c        |  2 +-
>   .../testing/selftests/kvm/memslot_perf_test.c |  2 +-
>   5 files changed, 45 insertions(+), 15 deletions(-)
> 

Looks good to me, thanks!

For the whole series:
Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

BTW: It looks like there was something wrong with the clock (or time zone
setup) of the machine this series was posted from since the "Date:"
headers on these four messages say they were sent Jun 8, 23:38 UTC
(while the time right now is Jun 8, 16:45 UTC).

Maciej
Paolo Bonzini June 8, 2021, 5:30 p.m. UTC | #2
On 09/06/21 01:38, Zhenzhong Duan wrote:
> (39fe2fc96694 "selftests: kvm: make allocation of extra memory take effect")
> changed the meaning of extra_mem_pages and treated it as slot0 memory size.
> 
> In fact extra_mem_pages is used for non-slot0 memory size, there is no custom
> slot0 memory size support. See discuss in https://lkml.org/lkml/2021/6/3/551
> for more details.
> 
> This patchset restores extra_mem_pages's original meaning and adds support for
> custom slot0 memory with a new parameter slot0_mem_pages.

Because the two reverts are so small, I squashed everything in a single 
patch with the following message:

     Until commit 39fe2fc96694 ("selftests: kvm: make allocation of extra
     memory take effect", 2021-05-27), parameter extra_mem_pages was used
     only to calculate the page table size for all the memory chunks,
     because real memory allocation happened with calls of
     vm_userspace_mem_region_add() after vm_create_default().

     Commit 39fe2fc96694 however changed the meaning of extra_mem_pages to
     the size of memory slot 0.  This makes the memory allocation more
     flexible, but makes it harder to account for the number of
     pages needed for the page tables.  For example, memslot_perf_test
     has a small amount of memory in slot 0 but a lot in other slots,
     and adding that memory twice (both in slot 0 and with later
     calls to vm_userspace_mem_region_add()) causes an error that
     was fixed in commit 000ac4295339 ("selftests: kvm: fix overlapping
     addresses in memslot_perf_test", 2021-05-29)

     Since both uses are sensible, add a new parameter slot0_mem_pages
     to vm_create_with_vcpus() and some comments to clarify the meaning of
     slot0_mem_pages and extra_mem_pages.  With this change,
     memslot_perf_test can go back to passing the number of memory
     pages as extra_mem_pages.

Paolo

> Run below command, all 39 tests passed.
> # make -C tools/testing/selftests/ TARGETS=kvm run_tests
> 
> Zhenzhong Duan (3):
>    Revert "selftests: kvm: make allocation of extra memory take effect"
>    Revert "selftests: kvm: fix overlapping addresses in
>      memslot_perf_test"
>    selftests: kvm: Add support for customized slot0 memory size
> 
>   .../testing/selftests/kvm/include/kvm_util.h  |  7 +--
>   .../selftests/kvm/kvm_page_table_test.c       |  2 +-
>   tools/testing/selftests/kvm/lib/kvm_util.c    | 47 +++++++++++++++----
>   .../selftests/kvm/lib/perf_test_util.c        |  2 +-
>   .../testing/selftests/kvm/memslot_perf_test.c |  2 +-
>   5 files changed, 45 insertions(+), 15 deletions(-)
>
Duan, Zhenzhong June 9, 2021, 5:19 a.m. UTC | #3
> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, June 9, 2021 1:30 AM
> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; linux-
> kernel@vger.kernel.org
> Cc: linux-kselftest@vger.kernel.org; kvm@vger.kernel.org;
> maciej.szmigiero@oracle.com; drjones@redhat.com; shuah@kernel.org
> Subject: Re: [PATCH 0/3] Restore extra_mem_pages and add
> slot0_mem_pages
> 
> On 09/06/21 01:38, Zhenzhong Duan wrote:
> > (39fe2fc96694 "selftests: kvm: make allocation of extra memory take
> > effect") changed the meaning of extra_mem_pages and treated it as slot0
> memory size.
> >
> > In fact extra_mem_pages is used for non-slot0 memory size, there is no
> > custom
> > slot0 memory size support. See discuss in
> > https://lkml.org/lkml/2021/6/3/551
> > for more details.
> >
> > This patchset restores extra_mem_pages's original meaning and adds
> > support for custom slot0 memory with a new parameter slot0_mem_pages.
> 
> Because the two reverts are so small, I squashed everything in a single patch
> with the following message:
> 
>      Until commit 39fe2fc96694 ("selftests: kvm: make allocation of extra
>      memory take effect", 2021-05-27), parameter extra_mem_pages was used
>      only to calculate the page table size for all the memory chunks,
>      because real memory allocation happened with calls of
>      vm_userspace_mem_region_add() after vm_create_default().
> 
>      Commit 39fe2fc96694 however changed the meaning of
> extra_mem_pages to
>      the size of memory slot 0.  This makes the memory allocation more
>      flexible, but makes it harder to account for the number of
>      pages needed for the page tables.  For example, memslot_perf_test
>      has a small amount of memory in slot 0 but a lot in other slots,
>      and adding that memory twice (both in slot 0 and with later
>      calls to vm_userspace_mem_region_add()) causes an error that
>      was fixed in commit 000ac4295339 ("selftests: kvm: fix overlapping
>      addresses in memslot_perf_test", 2021-05-29)
> 
>      Since both uses are sensible, add a new parameter slot0_mem_pages
>      to vm_create_with_vcpus() and some comments to clarify the meaning of
>      slot0_mem_pages and extra_mem_pages.  With this change,
>      memslot_perf_test can go back to passing the number of memory
>      pages as extra_mem_pages.
> 

This looks more clear, thanks for doing that.

Regards
Zhenzhong