Message ID | 20221014071914.227134-6-gshan@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp49051wrs; Fri, 14 Oct 2022 00:38:25 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7C2/uIHm3rttpcPSLMzT1e14hQIZO2r4z3qOgh+DsHYIxSQZWBlNdscSiLqukYxu7X1fj9 X-Received: by 2002:a63:7909:0:b0:458:1ba6:ec80 with SMTP id u9-20020a637909000000b004581ba6ec80mr3542135pgc.414.1665733105314; Fri, 14 Oct 2022 00:38:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1665733105; cv=none; d=google.com; s=arc-20160816; b=gdRFyi+sUnL6ckTWj+3+CN8yG+ALgcQtiWDEMNIoHwxGigQ3iXDK1U6CvlquPw7INh GmqWnpu9+P1lg2N7V726xAWt9Qr3qcKfI1mBsPQU5R34/CcSE/YyO/YEZKAYxXZ7prb/ EiNeuYhZiqahmxCp+6bo2NDD6nsIUvtBHqCJV2Z9B4BpDJGcOLx8EBB1sOm1B++eTEMP MMkn9lwsUPwdOphA6lAjz4MfHRWs4tHe+JA3s+ir1LtxgKtMETCn18UX5PAwapWXiGIQ 5odiA9vNhF3/yJELEtvUiyB8cQ/D/s9ExndjIn/hRPtwr9ZIrc9X6wAXv+efkcCpttbF be8Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=pr7VHnuYR01/Gc5/jPxyHFPhwmvdOLksQ25lnmxgeCs=; b=KtW8Mrki3nuSoKmv83f8z7PXL+cUQFp4lMQZ4LINtdK1GH8NiciehsvFfO9YAdNxCV QKGVZlpSL//quSHc2UtP3ONv8amq5Bi25ImCC6cL/vSDxHwOe7N0g71wxV1MU0rJbGcg F1KfsKBfcqh6NyH4rpcdfkqlcznf6h1Oc2Lj/vIVqSLLabIHVig6RcNPdcpjw+BbtaJh Tsh6z4MaEzIUO2cgn2kq9YkZ8QfCGupF7jOaDXfIGDRpLERtyy2ReBj7dOZ8Jz98ogBR c+cVsp6FePwzQ8AJmu6w9FvyTKYBrvlrwl+dFySC1BjBjRDxm7I5bje2SygkFDR5QSgl Cd1g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=f4UPCYCc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s5-20020a170902ea0500b0017f9b980fadsi2111954plg.446.2022.10.14.00.38.12; Fri, 14 Oct 2022 00:38:25 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=f4UPCYCc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229940AbiJNHVS (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Fri, 14 Oct 2022 03:21:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45064 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229732AbiJNHVO (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 14 Oct 2022 03:21:14 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A8919193744 for <linux-kernel@vger.kernel.org>; Fri, 14 Oct 2022 00:21:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665732073; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pr7VHnuYR01/Gc5/jPxyHFPhwmvdOLksQ25lnmxgeCs=; b=f4UPCYCclimsDtTFoFrtOAfMfJQLNIsfhvVqdlcYio/l9frLzsaGyYIg2zP64Gr+Nt0DQ3 L9y/fAMf4St4phe52nxhfQZE0EE2ZB6t9DpeWUvGK1mnTfn6cvXh/N5L0iYJZm3f3uf5Qx B305StfW9q5WZv9CPn+kqDXuJuezY/s= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-204-rtiAg9aKM9azsTkhDxYPzQ-1; Fri, 14 Oct 2022 03:21:08 -0400 X-MC-Unique: rtiAg9aKM9azsTkhDxYPzQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 972B51C05ED2; Fri, 14 Oct 2022 07:20:52 +0000 (UTC) Received: from gshan.redhat.com (vpn2-54-52.bne.redhat.com [10.64.54.52]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6FAF2C3343C; Fri, 14 Oct 2022 07:20:23 +0000 (UTC) From: Gavin Shan <gshan@redhat.com> To: kvmarm@lists.linux.dev Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, ajones@ventanamicro.com, pbonzini@redhat.com, maz@kernel.org, shuah@kernel.org, oliver.upton@linux.dev, seanjc@google.com, peterx@redhat.com, maciej.szmigiero@oracle.com, ricarkol@google.com, zhenyzha@redhat.com, shan.gavin@gmail.com Subject: [PATCH 5/6] KVM: selftests: memslot_perf_test: Consolidate memory sizes Date: Fri, 14 Oct 2022 15:19:13 +0800 Message-Id: <20221014071914.227134-6-gshan@redhat.com> In-Reply-To: <20221014071914.227134-1-gshan@redhat.com> References: <20221014071914.227134-1-gshan@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_NONE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1746647756806878193?= X-GMAIL-MSGID: =?utf-8?q?1746647756806878193?= |
Series |
KVM: selftests: memslot_perf_test: aarch64 cleanup/fixes
|
|
Commit Message
Gavin Shan
Oct. 14, 2022, 7:19 a.m. UTC
The addresses and sizes passed to madvise() and vm_userspace_mem_region_add()
should be aligned to host page size, which can be 64KB on aarch64. So it's
wrong by passing additional fixed 4KB memory area to various tests.
Fix it by passing additional fixed 64KB memory area to various tests. After
it's applied, the following command works fine on 64KB-page-size-host and
4KB-page-size-guest.
# ./memslot_perf_test -v -s 512
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
.../testing/selftests/kvm/memslot_perf_test.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
Comments
On 14.10.2022 09:19, Gavin Shan wrote: > The addresses and sizes passed to madvise() and vm_userspace_mem_region_add() > should be aligned to host page size, which can be 64KB on aarch64. So it's > wrong by passing additional fixed 4KB memory area to various tests. > > Fix it by passing additional fixed 64KB memory area to various tests. After > it's applied, the following command works fine on 64KB-page-size-host and > 4KB-page-size-guest. > > # ./memslot_perf_test -v -s 512 > > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > .../testing/selftests/kvm/memslot_perf_test.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c > index d587bd952ff9..e6d34744b45d 100644 > --- a/tools/testing/selftests/kvm/memslot_perf_test.c > +++ b/tools/testing/selftests/kvm/memslot_perf_test.c > @@ -25,12 +25,14 @@ > #include <kvm_util.h> > #include <processor.h> > > -#define MEM_SIZE ((512U << 20) + 4096) > -#define MEM_GPA 0x10000000UL > +#define MEM_EXTRA_SIZE 0x10000 So the biggest page size supported right now is 64 KiB - it would be good to have an assert somewhere to explicitly check for this (regardless of implicit checks present in other calculations). Also, an expression like "(64 << 10)" is more readable than a "1" with a tail of zeroes (it's easy to add one zero too many or be one zero short). Thanks, Maciej
On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: > > +#define MEM_EXTRA_SIZE 0x10000 > > Also, an expression like "(64 << 10)" is more readable than a "1" > with a tail of zeroes (it's easy to add one zero too many or be one > zero short). +1 to not open coding raw numbers. I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, 16KB, 64K, 2MB, 1GB, etc... Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do math off of those.
On 10/18/22 6:08 AM, Sean Christopherson wrote: > On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: >>> +#define MEM_EXTRA_SIZE 0x10000 >> >> Also, an expression like "(64 << 10)" is more readable than a "1" >> with a tail of zeroes (it's easy to add one zero too many or be one >> zero short). > > +1 to not open coding raw numbers. > > I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, > 16KB, 64K, 2MB, 1GB, etc... > > Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do > math off of those. > Ok. I will have one separate patch to define those sizes in kvm_util_base.h, right after '#define NSEC_PER_SEC 1000000000L'. Sean, could you let me know if it looks good to you? #define KB (1UL << 10) #define MB (1UL << 20) #define GB (1UL << 30) #define TB (1UL << 40) /* Base page and huge page size */ #define SIZE_4KB ( 4 * KB) #define SIZE_16KB ( 16 * KB) #define SIZE_64KB ( 64 * KB) #define SIZE_2MB ( 2 * MB) #define SIZE_32MB ( 32 * MB) #define SIZE_512MB (512 * MB) #define SIZE_1GB ( 1 * GB) #define SIZE_16GB ( 16 * GB) Thanks, Gavin
On 18.10.2022 00:51, Gavin Shan wrote: > On 10/18/22 6:08 AM, Sean Christopherson wrote: >> On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: >>>> +#define MEM_EXTRA_SIZE 0x10000 >>> >>> Also, an expression like "(64 << 10)" is more readable than a "1" >>> with a tail of zeroes (it's easy to add one zero too many or be one >>> zero short). >> >> +1 to not open coding raw numbers. >> >> I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, >> 16KB, 64K, 2MB, 1GB, etc... >> >> Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do >> math off of those. >> > > Ok. I will have one separate patch to define those sizes in kvm_util_base.h, > right after '#define NSEC_PER_SEC 1000000000L'. Sean, could you let me know > if it looks good to you? > > #define KB (1UL << 10) > #define MB (1UL << 20) > #define GB (1UL << 30) > #define TB (1UL << 40) > > /* Base page and huge page size */ > #define SIZE_4KB ( 4 * KB) > #define SIZE_16KB ( 16 * KB) > #define SIZE_64KB ( 64 * KB) > #define SIZE_2MB ( 2 * MB) > #define SIZE_32MB ( 32 * MB) > #define SIZE_512MB (512 * MB) > #define SIZE_1GB ( 1 * GB) > #define SIZE_16GB ( 16 * GB) FYI, QEMU uses KiB, MiB, GiB, etc., see [1]. > Thanks, > Gavin > Thanks, Maciej [1]: https://git.qemu.org/?p=qemu.git;a=blob;f=include/qemu/units.h;hb=HEAD
On 10/18/22 6:56 AM, Maciej S. Szmigiero wrote: > On 18.10.2022 00:51, Gavin Shan wrote: >> On 10/18/22 6:08 AM, Sean Christopherson wrote: >>> On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: >>>>> +#define MEM_EXTRA_SIZE 0x10000 >>>> >>>> Also, an expression like "(64 << 10)" is more readable than a "1" >>>> with a tail of zeroes (it's easy to add one zero too many or be one >>>> zero short). >>> >>> +1 to not open coding raw numbers. >>> >>> I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, >>> 16KB, 64K, 2MB, 1GB, etc... >>> >>> Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do >>> math off of those. >>> >> >> Ok. I will have one separate patch to define those sizes in kvm_util_base.h, >> right after '#define NSEC_PER_SEC 1000000000L'. Sean, could you let me know >> if it looks good to you? >> >> #define KB (1UL << 10) >> #define MB (1UL << 20) >> #define GB (1UL << 30) >> #define TB (1UL << 40) >> >> /* Base page and huge page size */ >> #define SIZE_4KB ( 4 * KB) >> #define SIZE_16KB ( 16 * KB) >> #define SIZE_64KB ( 64 * KB) >> #define SIZE_2MB ( 2 * MB) >> #define SIZE_32MB ( 32 * MB) >> #define SIZE_512MB (512 * MB) >> #define SIZE_1GB ( 1 * GB) >> #define SIZE_16GB ( 16 * GB) > > FYI, QEMU uses KiB, MiB, GiB, etc., see [1]. > Right. I checked QEMU's definitions and it makes sense to use KiB, MiB, GiB, TiB. I don't think we need PiB and EiB because our tests don't use that large memory. > > [1]: https://git.qemu.org/?p=qemu.git;a=blob;f=include/qemu/units.h;hb=HEAD > Thanks, Gavin
On Tue, Oct 18, 2022, Gavin Shan wrote: > On 10/18/22 6:56 AM, Maciej S. Szmigiero wrote: > > On 18.10.2022 00:51, Gavin Shan wrote: > > > On 10/18/22 6:08 AM, Sean Christopherson wrote: > > > > On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: > > > > > > +#define MEM_EXTRA_SIZE 0x10000 > > > > > > > > > > Also, an expression like "(64 << 10)" is more readable than a "1" > > > > > with a tail of zeroes (it's easy to add one zero too many or be one > > > > > zero short). > > > > > > > > +1 to not open coding raw numbers. > > > > > > > > I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, > > > > 16KB, 64K, 2MB, 1GB, etc... > > > > > > > > Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do > > > > math off of those. > > > > > > > > > > Ok. I will have one separate patch to define those sizes in kvm_util_base.h, > > > right after '#define NSEC_PER_SEC 1000000000L'. Sean, could you let me know > > > if it looks good to you? > > > > > > #define KB (1UL << 10) > > > #define MB (1UL << 20) > > > #define GB (1UL << 30) > > > #define TB (1UL << 40) Any objection to prefixing these with SIZE_ as well? IMO it's worth burning the extra five characters to make it all but impossible to misinterpret code. > > > /* Base page and huge page size */ > > > #define SIZE_4KB ( 4 * KB) > > > #define SIZE_16KB ( 16 * KB) > > > #define SIZE_64KB ( 64 * KB) > > > #define SIZE_2MB ( 2 * MB) > > > #define SIZE_32MB ( 32 * MB) > > > #define SIZE_512MB (512 * MB) > > > #define SIZE_1GB ( 1 * GB) > > > #define SIZE_16GB ( 16 * GB) > > > > FYI, QEMU uses KiB, MiB, GiB, etc., see [1]. > > > > Right. I checked QEMU's definitions and it makes sense to use > KiB, MiB, GiB, TiB. I don't think we need PiB and EiB because > our tests don't use that large memory. Ha! I had typed out KiB, etc... but then thought, "nah, I'm being silly". KiB and friends work for me.
On 10/18/22 7:32 AM, Sean Christopherson wrote: > On Tue, Oct 18, 2022, Gavin Shan wrote: >> On 10/18/22 6:56 AM, Maciej S. Szmigiero wrote: >>> On 18.10.2022 00:51, Gavin Shan wrote: >>>> On 10/18/22 6:08 AM, Sean Christopherson wrote: >>>>> On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: >>>>>>> +#define MEM_EXTRA_SIZE 0x10000 >>>>>> >>>>>> Also, an expression like "(64 << 10)" is more readable than a "1" >>>>>> with a tail of zeroes (it's easy to add one zero too many or be one >>>>>> zero short). >>>>> >>>>> +1 to not open coding raw numbers. >>>>> >>>>> I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, >>>>> 16KB, 64K, 2MB, 1GB, etc... >>>>> >>>>> Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do >>>>> math off of those. >>>>> >>>> >>>> Ok. I will have one separate patch to define those sizes in kvm_util_base.h, >>>> right after '#define NSEC_PER_SEC 1000000000L'. Sean, could you let me know >>>> if it looks good to you? >>>> >>>> #define KB (1UL << 10) >>>> #define MB (1UL << 20) >>>> #define GB (1UL << 30) >>>> #define TB (1UL << 40) > > Any objection to prefixing these with SIZE_ as well? IMO it's worth burning the > extra five characters to make it all but impossible to misinterpret code. > 'SIZE_' prefix works for me either. >>>> /* Base page and huge page size */ >>>> #define SIZE_4KB ( 4 * KB) >>>> #define SIZE_16KB ( 16 * KB) >>>> #define SIZE_64KB ( 64 * KB) >>>> #define SIZE_2MB ( 2 * MB) >>>> #define SIZE_32MB ( 32 * MB) >>>> #define SIZE_512MB (512 * MB) >>>> #define SIZE_1GB ( 1 * GB) >>>> #define SIZE_16GB ( 16 * GB) >>> >>> FYI, QEMU uses KiB, MiB, GiB, etc., see [1]. >>> >> >> Right. I checked QEMU's definitions and it makes sense to use >> KiB, MiB, GiB, TiB. I don't think we need PiB and EiB because >> our tests don't use that large memory. > > Ha! I had typed out KiB, etc... but then thought, "nah, I'm being silly". KiB > and friends work for me. > Thanks for your confirm, Sean. Thanks, Gavin
On 10/18/22 5:36 AM, Maciej S. Szmigiero wrote: > On 14.10.2022 09:19, Gavin Shan wrote: >> The addresses and sizes passed to madvise() and vm_userspace_mem_region_add() >> should be aligned to host page size, which can be 64KB on aarch64. So it's >> wrong by passing additional fixed 4KB memory area to various tests. >> >> Fix it by passing additional fixed 64KB memory area to various tests. After >> it's applied, the following command works fine on 64KB-page-size-host and >> 4KB-page-size-guest. >> >> # ./memslot_perf_test -v -s 512 >> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> .../testing/selftests/kvm/memslot_perf_test.c | 18 ++++++++++-------- >> 1 file changed, 10 insertions(+), 8 deletions(-) >> >> diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c >> index d587bd952ff9..e6d34744b45d 100644 >> --- a/tools/testing/selftests/kvm/memslot_perf_test.c >> +++ b/tools/testing/selftests/kvm/memslot_perf_test.c >> @@ -25,12 +25,14 @@ >> #include <kvm_util.h> >> #include <processor.h> >> -#define MEM_SIZE ((512U << 20) + 4096) >> -#define MEM_GPA 0x10000000UL >> +#define MEM_EXTRA_SIZE 0x10000 > > So the biggest page size supported right now is 64 KiB - it would be > good to have an assert somewhere to explicitly check for this > (regardless of implicit checks present in other calculations). > > Also, an expression like "(64 << 10)" is more readable than a "1" > with a tail of zeroes (it's easy to add one zero too many or be one > zero short). > Yes, it makes sense to me. Lets add check in check_memory_sizes(), which was added in the previous patch, to fail early if host/guest page size exceeds 64KB. if (host_page_size > SIZE_64KiB || guest_page_size > SIZE_64KiB) { pr_info("Unsupported page size on host (0x%x) or guest (0x%x)\n", host_page_size, guest_page_size); } For the macros, I think all of us agree on KiB, MiB, GiB, TiB and their variants :) Thanks, Gavin
On Mon, Oct 17, 2022 at 10:08:48PM +0000, Sean Christopherson wrote: > On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: > > > +#define MEM_EXTRA_SIZE 0x10000 > > > > Also, an expression like "(64 << 10)" is more readable than a "1" > > with a tail of zeroes (it's easy to add one zero too many or be one > > zero short). > > +1 to not open coding raw numbers. > > I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, > 16KB, 64K, 2MB, 1GB, etc... > > Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do > math off of those. I mean I love boilerplate as much as the next guy, but we can just use tools/include/linux/sizes.h -- Thanks, Oliver
On 10/18/22 3:47 PM, Oliver Upton wrote: > On Mon, Oct 17, 2022 at 10:08:48PM +0000, Sean Christopherson wrote: >> On Mon, Oct 17, 2022, Maciej S. Szmigiero wrote: >>>> +#define MEM_EXTRA_SIZE 0x10000 >>> >>> Also, an expression like "(64 << 10)" is more readable than a "1" >>> with a tail of zeroes (it's easy to add one zero too many or be one >>> zero short). >> >> +1 to not open coding raw numbers. >> >> I think it's high time KVM selftests add #defines for the common sizes, e.g. SIZE_4KB, >> 16KB, 64K, 2MB, 1GB, etc... >> >> Alternatively (or in addition), just #define 1KB, 1MB, 1GB, and 1TB, and then do >> math off of those. > > I mean I love boilerplate as much as the next guy, but we can just use > tools/include/linux/sizes.h > Nice point, I didn't realize we already had 'tools/include/linux/sizes.h'. The suggested macros (KiB, MiB, GiB, TiB and their variants) have been added to PATCH[v2 5/6]. I think it's reasonable to use 'tools/include/linux/sizes.h' directly instead of reinventing the wheel. I will go ahead to use 'tools/include/linux/sizes.h' directly in v3 if nobody objects. I would like to receive comments on v2 before I'm going to post v3. Thanks, Gavin
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c index d587bd952ff9..e6d34744b45d 100644 --- a/tools/testing/selftests/kvm/memslot_perf_test.c +++ b/tools/testing/selftests/kvm/memslot_perf_test.c @@ -25,12 +25,14 @@ #include <kvm_util.h> #include <processor.h> -#define MEM_SIZE ((512U << 20) + 4096) -#define MEM_GPA 0x10000000UL +#define MEM_EXTRA_SIZE 0x10000 + +#define MEM_SIZE ((512U << 20) + MEM_EXTRA_SIZE) +#define MEM_GPA 0x10000000UL #define MEM_AUX_GPA MEM_GPA #define MEM_SYNC_GPA MEM_AUX_GPA -#define MEM_TEST_GPA (MEM_AUX_GPA + 4096) -#define MEM_TEST_SIZE (MEM_SIZE - 4096) +#define MEM_TEST_GPA (MEM_AUX_GPA + MEM_EXTRA_SIZE) +#define MEM_TEST_SIZE (MEM_SIZE - MEM_EXTRA_SIZE) /* * 32 MiB is max size that gets well over 100 iterations on 509 slots. @@ -38,8 +40,8 @@ * 8194 slots in use can then be tested (although with slightly * limited resolution). */ -#define MEM_SIZE_MAP ((32U << 20) + 4096) -#define MEM_TEST_MAP_SIZE (MEM_SIZE_MAP - 4096) +#define MEM_SIZE_MAP ((32U << 20) + MEM_EXTRA_SIZE) +#define MEM_TEST_MAP_SIZE (MEM_SIZE_MAP - MEM_EXTRA_SIZE) /* * 128 MiB is min size that fills 32k slots with at least one page in each @@ -799,13 +801,13 @@ static const struct test_data tests[] = { }, { .name = "unmap", - .mem_size = MEM_TEST_UNMAP_SIZE + 4096, + .mem_size = MEM_TEST_UNMAP_SIZE + MEM_EXTRA_SIZE, .guest_code = guest_code_test_memslot_unmap, .loop = test_memslot_unmap_loop, }, { .name = "unmap chunked", - .mem_size = MEM_TEST_UNMAP_SIZE + 4096, + .mem_size = MEM_TEST_UNMAP_SIZE + MEM_EXTRA_SIZE, .guest_code = guest_code_test_memslot_unmap, .loop = test_memslot_unmap_loop_chunked, },