From: David Matlack <dmatlack@google.com>
To: Ben Gardon <bgardon@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
kvm list <kvm@vger.kernel.org>,
Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>,
Sean Christopherson <seanjc@google.com>,
Peter Shier <pshier@google.com>,
David Dunn <daviddunn@google.com>,
Junaid Shahid <junaids@google.com>,
Jim Mattson <jmattson@google.com>,
Mingwei Zhang <mizhang@google.com>,
Jing Zhang <jingzhangos@google.com>
Subject: Re: [PATCH v4 01/10] KVM: selftests: Remove dynamic memory allocation for stats header
Date: Mon, 11 Apr 2022 14:52:24 -0700 [thread overview]
Message-ID: <CALzav=d_DB+onkMZmZwEj7yd_gDqg_phqQJpTw0g9ahe3WmvqA@mail.gmail.com> (raw)
In-Reply-To: <20220411211015.3091615-2-bgardon@google.com>
On Mon, Apr 11, 2022 at 2:10 PM Ben Gardon <bgardon@google.com> wrote:
>
> There's no need to allocate dynamic memory for the stats header since
> its size is known at compile time.
>
> Signed-off-by: Ben Gardon <bgardon@google.com>
Reviewed-by: David Matlack <dmatlack@google.com>
> ---
> .../selftests/kvm/kvm_binary_stats_test.c | 58 +++++++++----------
> 1 file changed, 27 insertions(+), 31 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
> index 17f65d514915..dad34d8a41fe 100644
> --- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
> +++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
> @@ -26,56 +26,53 @@ static void stats_test(int stats_fd)
> int i;
> size_t size_desc;
> size_t size_data = 0;
> - struct kvm_stats_header *header;
> + struct kvm_stats_header header;
> char *id;
> struct kvm_stats_desc *stats_desc;
> u64 *stats_data;
> struct kvm_stats_desc *pdesc;
>
> /* Read kvm stats header */
> - header = malloc(sizeof(*header));
> - TEST_ASSERT(header, "Allocate memory for stats header");
> -
> - ret = read(stats_fd, header, sizeof(*header));
> - TEST_ASSERT(ret == sizeof(*header), "Read stats header");
> - size_desc = sizeof(*stats_desc) + header->name_size;
> + ret = read(stats_fd, &header, sizeof(header));
> + TEST_ASSERT(ret == sizeof(header), "Read stats header");
> + size_desc = sizeof(*stats_desc) + header.name_size;
>
> /* Read kvm stats id string */
> - id = malloc(header->name_size);
> + id = malloc(header.name_size);
> TEST_ASSERT(id, "Allocate memory for id string");
> - ret = read(stats_fd, id, header->name_size);
> - TEST_ASSERT(ret == header->name_size, "Read id string");
> + ret = read(stats_fd, id, header.name_size);
> + TEST_ASSERT(ret == header.name_size, "Read id string");
>
> /* Check id string, that should start with "kvm" */
> - TEST_ASSERT(!strncmp(id, "kvm", 3) && strlen(id) < header->name_size,
> + TEST_ASSERT(!strncmp(id, "kvm", 3) && strlen(id) < header.name_size,
> "Invalid KVM stats type, id: %s", id);
>
> /* Sanity check for other fields in header */
> - if (header->num_desc == 0) {
> + if (header.num_desc == 0) {
> printf("No KVM stats defined!");
> return;
> }
> /* Check overlap */
> - TEST_ASSERT(header->desc_offset > 0 && header->data_offset > 0
> - && header->desc_offset >= sizeof(*header)
> - && header->data_offset >= sizeof(*header),
> + TEST_ASSERT(header.desc_offset > 0 && header.data_offset > 0
> + && header.desc_offset >= sizeof(header)
> + && header.data_offset >= sizeof(header),
> "Invalid offset fields in header");
> - TEST_ASSERT(header->desc_offset > header->data_offset ||
> - (header->desc_offset + size_desc * header->num_desc <=
> - header->data_offset),
> + TEST_ASSERT(header.desc_offset > header.data_offset ||
> + (header.desc_offset + size_desc * header.num_desc <=
> + header.data_offset),
> "Descriptor block is overlapped with data block");
>
> /* Allocate memory for stats descriptors */
> - stats_desc = calloc(header->num_desc, size_desc);
> + stats_desc = calloc(header.num_desc, size_desc);
> TEST_ASSERT(stats_desc, "Allocate memory for stats descriptors");
> /* Read kvm stats descriptors */
> ret = pread(stats_fd, stats_desc,
> - size_desc * header->num_desc, header->desc_offset);
> - TEST_ASSERT(ret == size_desc * header->num_desc,
> + size_desc * header.num_desc, header.desc_offset);
> + TEST_ASSERT(ret == size_desc * header.num_desc,
> "Read KVM stats descriptors");
>
> /* Sanity check for fields in descriptors */
> - for (i = 0; i < header->num_desc; ++i) {
> + for (i = 0; i < header.num_desc; ++i) {
> pdesc = (void *)stats_desc + i * size_desc;
> /* Check type,unit,base boundaries */
> TEST_ASSERT((pdesc->flags & KVM_STATS_TYPE_MASK)
> @@ -104,7 +101,7 @@ static void stats_test(int stats_fd)
> break;
> }
> /* Check name string */
> - TEST_ASSERT(strlen(pdesc->name) < header->name_size,
> + TEST_ASSERT(strlen(pdesc->name) < header.name_size,
> "KVM stats name(%s) too long", pdesc->name);
> /* Check size field, which should not be zero */
> TEST_ASSERT(pdesc->size, "KVM descriptor(%s) with size of 0",
> @@ -124,14 +121,14 @@ static void stats_test(int stats_fd)
> size_data += pdesc->size * sizeof(*stats_data);
> }
> /* Check overlap */
> - TEST_ASSERT(header->data_offset >= header->desc_offset
> - || header->data_offset + size_data <= header->desc_offset,
> + TEST_ASSERT(header.data_offset >= header.desc_offset
> + || header.data_offset + size_data <= header.desc_offset,
> "Data block is overlapped with Descriptor block");
> /* Check validity of all stats data size */
> - TEST_ASSERT(size_data >= header->num_desc * sizeof(*stats_data),
> + TEST_ASSERT(size_data >= header.num_desc * sizeof(*stats_data),
> "Data size is not correct");
> /* Check stats offset */
> - for (i = 0; i < header->num_desc; ++i) {
> + for (i = 0; i < header.num_desc; ++i) {
> pdesc = (void *)stats_desc + i * size_desc;
> TEST_ASSERT(pdesc->offset < size_data,
> "Invalid offset (%u) for stats: %s",
> @@ -142,15 +139,15 @@ static void stats_test(int stats_fd)
> stats_data = malloc(size_data);
> TEST_ASSERT(stats_data, "Allocate memory for stats data");
> /* Read kvm stats data as a bulk */
> - ret = pread(stats_fd, stats_data, size_data, header->data_offset);
> + ret = pread(stats_fd, stats_data, size_data, header.data_offset);
> TEST_ASSERT(ret == size_data, "Read KVM stats data");
> /* Read kvm stats data one by one */
> size_data = 0;
> - for (i = 0; i < header->num_desc; ++i) {
> + for (i = 0; i < header.num_desc; ++i) {
> pdesc = (void *)stats_desc + i * size_desc;
> ret = pread(stats_fd, stats_data,
> pdesc->size * sizeof(*stats_data),
> - header->data_offset + size_data);
> + header.data_offset + size_data);
> TEST_ASSERT(ret == pdesc->size * sizeof(*stats_data),
> "Read data of KVM stats: %s", pdesc->name);
> size_data += pdesc->size * sizeof(*stats_data);
> @@ -159,7 +156,6 @@ static void stats_test(int stats_fd)
> free(stats_data);
> free(stats_desc);
> free(id);
> - free(header);
> }
>
>
> --
> 2.35.1.1178.g4f1659d476-goog
>
next prev parent reply other threads:[~2022-04-11 21:52 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-11 21:10 [PATCH v4 00/10] KVM: x86: Add a cap to disable NX hugepages on a VM Ben Gardon
2022-04-11 21:10 ` [PATCH v4 01/10] KVM: selftests: Remove dynamic memory allocation for stats header Ben Gardon
2022-04-11 21:52 ` David Matlack [this message]
2022-04-11 22:50 ` Mingwei Zhang
2022-04-11 21:10 ` [PATCH v4 02/10] KVM: selftests: Read binary stats header in lib Ben Gardon
2022-04-11 21:55 ` David Matlack
2022-04-11 21:10 ` [PATCH v4 03/10] KVM: selftests: Read binary stats desc " Ben Gardon
2022-04-11 22:01 ` David Matlack
2022-04-12 0:54 ` Mingwei Zhang
2022-04-12 18:56 ` Ben Gardon
2022-04-12 19:02 ` Sean Christopherson
2022-04-12 20:02 ` Sean Christopherson
2022-04-12 22:12 ` Ben Gardon
2022-04-11 21:10 ` [PATCH v4 04/10] KVM: selftests: Read binary stat data " Ben Gardon
2022-04-11 22:14 ` David Matlack
2022-04-12 19:58 ` Ben Gardon
2022-04-12 1:25 ` Mingwei Zhang
2022-04-11 21:10 ` [PATCH v4 05/10] KVM: selftests: Add NX huge pages test Ben Gardon
2022-04-11 22:27 ` David Matlack
2022-04-12 22:11 ` Ben Gardon
2022-04-12 1:32 ` Mingwei Zhang
2022-04-12 21:51 ` Ben Gardon
2022-04-11 21:10 ` [PATCH v4 06/10] KVM: x86/MMU: Factor out updating NX hugepages state for a VM Ben Gardon
2022-04-11 21:10 ` [PATCH v4 07/10] KVM: x86/MMU: Allow NX huge pages to be disabled on a per-vm basis Ben Gardon
2022-04-12 17:54 ` Sean Christopherson
2022-04-11 21:10 ` [PATCH v4 08/10] KVM: x86: Fix errant brace in KVM capability handling Ben Gardon
2022-04-11 21:10 ` [PATCH v4 09/10] KVM: x86/MMU: Require reboot permission to disable NX hugepages Ben Gardon
2022-04-12 18:08 ` Sean Christopherson
2022-04-11 21:10 ` [PATCH v4 10/10] KVM: selftests: Test disabling NX hugepages on a VM Ben Gardon
2022-04-11 22:37 ` David Matlack
2022-04-11 21:15 ` [PATCH v4 00/10] KVM: x86: Add a cap to disable " Ben Gardon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CALzav=d_DB+onkMZmZwEj7yd_gDqg_phqQJpTw0g9ahe3WmvqA@mail.gmail.com' \
--to=dmatlack@google.com \
--cc=bgardon@google.com \
--cc=daviddunn@google.com \
--cc=jingzhangos@google.com \
--cc=jmattson@google.com \
--cc=junaids@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mizhang@google.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=pshier@google.com \
--cc=seanjc@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.