All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Vipin Sharma <vipinsh@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	kvm-riscv@lists.infradead.org, seanjc@google.com,
	pbonzini@redhat.com, borntraeger@linux.ibm.com,
	frankja@linux.ibm.com, imbrenda@linux.ibm.com,
	anup@brainfault.org, atish.patra@linux.dev,
	zhaotianrui@loongson.cn, maobibo@loongson.cn,
	chenhuacai@kernel.org, oliver.upton@linux.dev,
	ajones@ventanamicro.com
Subject: Re: [PATCH v3 9/9] KVM: selftests: Provide README.rst for KVM selftests runner
Date: Thu, 02 Oct 2025 15:41:26 +0100	[thread overview]
Message-ID: <86a529z7qh.wl-maz@kernel.org> (raw)
In-Reply-To: <20251001173225.GA420255.vipinsh@google.com>

On Wed, 01 Oct 2025 18:32:25 +0100,
Vipin Sharma <vipinsh@google.com> wrote:
> 
> On 2025-10-01 09:44:22, Marc Zyngier wrote:
> > On Tue, 30 Sep 2025 17:36:35 +0100,
> > Vipin Sharma <vipinsh@google.com> wrote:
> > > 
> > > +KVM selftest runner is highly configurable test executor that allows to run
> > > +tests with different configurations (not just the default), parallely, save
> > 
> > s/parallely/in parallel/
> > 
> 
> Thanks, I will fix it.
> 
> > > +output to disk hierarchically, control what gets printed on console, provide
> > > +execution status.
> > > +
> > > +To generate default tests use::
> > > +
> > > +  # make tests_install
> > > +
> > > +This will create ``testcases_default_gen`` directory which will have testcases
> > 
> > I don't think using the future tense is correct here. I'd rather see
> > something written in the present tense, possibly imperative. For
> > example:
> > 
> > "Create 'blah' directory containing 'foo' files, one per test-case.
> > 
> 
> Thanks, I will fix it.
> 
> > > +in `default.test` files. Each KVM selftest will have a directory in  which
> > > +`default.test` file will be created with executable path relative to KVM
> > > +selftest root directory i.e. `/tools/testing/selftests/kvm`.
> > 
> > Shouldn't this honor the existing build output directives? If it
> > actually does, then you want to call this out.
> > 
> 
> To generate default test files in a specific directory one can use
> "OUTPUT" in the make command

The standard way to do this is documented in the top level Makefile:

<quote>
# This does not need to match to the root of the kernel source tree.
#
# For example, you can do this:
#
#  cd /dir/to/store/output/files; make -f /dir/to/kernel/source/Makefile
#
# If you want to save output files in a different location, there are
# two syntaxes to specify it.
#
# 1) O=
# Use "make O=dir/to/store/output/files/"
#
# 2) Set KBUILD_OUTPUT
# Set the environment variable KBUILD_OUTPUT to point to the output directory.
# export KBUILD_OUTPUT=dir/to/store/output/files/; make
#
# The O= assignment takes precedence over the KBUILD_OUTPUT environment
# variable.
</quote>

Your new infrastructure should support the existing mechanism (and
avoid introducing a new one).

> 
>   make OUTPUT="~/test/directory/path" tests_install
> 
> This allows to generate testcases_default_gen in the given output
> directory. default.test files will still have test binary path relative
> kvm selftest root directory. 
> 
> $OUTPUT
> └── testcases_default_gen
>     ├── access_tracking_perf_test
>     │   └── default.test
>     ├── arch_timer
>     │   └── default.test
>     ├── arm64
>     │   ├── aarch32_id_regs
>     │   │   └── default.test
>     │   ├── arch_timer_edge_cases
>     │   │   └── default.test
>     │   ├── debug-exceptions
>     │   │   └── default.test
>     │   ├── external_aborts
>     │   │   └── default.test
>     │   │   └── default.test
>     │   └── ...
>     ├── coalesced_io_test
>     │   └── default.test
>     ├── demand_paging_test
>     │   └── default.test
>     ├── ...
> 
> So, arm64/aarch32_id_regs/default.test will have 'arm64/aarch32_id_regs'
> 
> User can then supply -p/--path with the path of build output directory
> to runner. 
> 
>   python3 runner -p ~/path/to/selftest/binaries -d $OUTPUT/testcases_default_gen
> 
> If -p not given then current directory is considered for test
> executables.
>
> > > For example, the
> > > +`dirty_log_perf_test` will have::
> > > +
> > > +  # cat testcase_default_gen/dirty_log_perf_test/default.test
> > > +  dirty_log_perf_test
> > > +
> > > +Runner will execute `dirty_log_perf_test`. Testcases files can also provide
> > > +extra arguments to the test::
> > > +
> > > +  # cat tests/dirty_log_perf_test/2slot_5vcpu_10iter.test
> > > +  dirty_log_perf_test -x 2 -v 5 -i 10
> > > +
> > > +In this case runner will execute the `dirty_log_perf_test` with the options.
> > > +
> > 
> > The beginning of the text talks about "non-default' configurations,
> > but you only seem to talk about the default stuff. How does one deals
> > with a non-default config?
> > 
> 
> In the patch 1, I created two sample tests files,
> 2slot_5vcpu_10iter.test and no_dirty_log_protect.test, in the directory
> tools/testing/selftests/kvm/tests/dirty_log_perf_test.
> 
> Contents of those files provide non-default arguments to test, for example,
> 2slot_5vcpu10iter.test has the command:
> 
>   dirty_log_perf_test -x 2 -v 5  -i 10
> 
> One can run these non-default tests as (assuming current directory is
> kvm selftests):
> 
>   python3 runner -d ./tests
> 
> Over the time we will add more of these non-default interesting
> testcases. One can then run:
> 
>   python3 runner -d ./tests ./testcases_default_gen

That's not what I am complaining about. What you call "configuration"
seems to just be "random set of parameters for a random test".

In practice, your runner does not seem configurable at all. You just
treat all possible configurations of a single test as different tests.

My (admittedly very personal) view of what a configuration should be
is "run this single test with these parameters varying in these
ranges, for this long".

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Vipin Sharma <vipinsh@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	kvm-riscv@lists.infradead.org, seanjc@google.com,
	pbonzini@redhat.com, borntraeger@linux.ibm.com,
	frankja@linux.ibm.com, imbrenda@linux.ibm.com,
	anup@brainfault.org, atish.patra@linux.dev,
	zhaotianrui@loongson.cn, maobibo@loongson.cn,
	chenhuacai@kernel.org, oliver.upton@linux.dev,
	ajones@ventanamicro.com
Subject: Re: [PATCH v3 9/9] KVM: selftests: Provide README.rst for KVM selftests runner
Date: Thu, 02 Oct 2025 15:41:26 +0100	[thread overview]
Message-ID: <86a529z7qh.wl-maz@kernel.org> (raw)
In-Reply-To: <20251001173225.GA420255.vipinsh@google.com>

On Wed, 01 Oct 2025 18:32:25 +0100,
Vipin Sharma <vipinsh@google.com> wrote:
> 
> On 2025-10-01 09:44:22, Marc Zyngier wrote:
> > On Tue, 30 Sep 2025 17:36:35 +0100,
> > Vipin Sharma <vipinsh@google.com> wrote:
> > > 
> > > +KVM selftest runner is highly configurable test executor that allows to run
> > > +tests with different configurations (not just the default), parallely, save
> > 
> > s/parallely/in parallel/
> > 
> 
> Thanks, I will fix it.
> 
> > > +output to disk hierarchically, control what gets printed on console, provide
> > > +execution status.
> > > +
> > > +To generate default tests use::
> > > +
> > > +  # make tests_install
> > > +
> > > +This will create ``testcases_default_gen`` directory which will have testcases
> > 
> > I don't think using the future tense is correct here. I'd rather see
> > something written in the present tense, possibly imperative. For
> > example:
> > 
> > "Create 'blah' directory containing 'foo' files, one per test-case.
> > 
> 
> Thanks, I will fix it.
> 
> > > +in `default.test` files. Each KVM selftest will have a directory in  which
> > > +`default.test` file will be created with executable path relative to KVM
> > > +selftest root directory i.e. `/tools/testing/selftests/kvm`.
> > 
> > Shouldn't this honor the existing build output directives? If it
> > actually does, then you want to call this out.
> > 
> 
> To generate default test files in a specific directory one can use
> "OUTPUT" in the make command

The standard way to do this is documented in the top level Makefile:

<quote>
# This does not need to match to the root of the kernel source tree.
#
# For example, you can do this:
#
#  cd /dir/to/store/output/files; make -f /dir/to/kernel/source/Makefile
#
# If you want to save output files in a different location, there are
# two syntaxes to specify it.
#
# 1) O=
# Use "make O=dir/to/store/output/files/"
#
# 2) Set KBUILD_OUTPUT
# Set the environment variable KBUILD_OUTPUT to point to the output directory.
# export KBUILD_OUTPUT=dir/to/store/output/files/; make
#
# The O= assignment takes precedence over the KBUILD_OUTPUT environment
# variable.
</quote>

Your new infrastructure should support the existing mechanism (and
avoid introducing a new one).

> 
>   make OUTPUT="~/test/directory/path" tests_install
> 
> This allows to generate testcases_default_gen in the given output
> directory. default.test files will still have test binary path relative
> kvm selftest root directory. 
> 
> $OUTPUT
> └── testcases_default_gen
>     ├── access_tracking_perf_test
>     │   └── default.test
>     ├── arch_timer
>     │   └── default.test
>     ├── arm64
>     │   ├── aarch32_id_regs
>     │   │   └── default.test
>     │   ├── arch_timer_edge_cases
>     │   │   └── default.test
>     │   ├── debug-exceptions
>     │   │   └── default.test
>     │   ├── external_aborts
>     │   │   └── default.test
>     │   │   └── default.test
>     │   └── ...
>     ├── coalesced_io_test
>     │   └── default.test
>     ├── demand_paging_test
>     │   └── default.test
>     ├── ...
> 
> So, arm64/aarch32_id_regs/default.test will have 'arm64/aarch32_id_regs'
> 
> User can then supply -p/--path with the path of build output directory
> to runner. 
> 
>   python3 runner -p ~/path/to/selftest/binaries -d $OUTPUT/testcases_default_gen
> 
> If -p not given then current directory is considered for test
> executables.
>
> > > For example, the
> > > +`dirty_log_perf_test` will have::
> > > +
> > > +  # cat testcase_default_gen/dirty_log_perf_test/default.test
> > > +  dirty_log_perf_test
> > > +
> > > +Runner will execute `dirty_log_perf_test`. Testcases files can also provide
> > > +extra arguments to the test::
> > > +
> > > +  # cat tests/dirty_log_perf_test/2slot_5vcpu_10iter.test
> > > +  dirty_log_perf_test -x 2 -v 5 -i 10
> > > +
> > > +In this case runner will execute the `dirty_log_perf_test` with the options.
> > > +
> > 
> > The beginning of the text talks about "non-default' configurations,
> > but you only seem to talk about the default stuff. How does one deals
> > with a non-default config?
> > 
> 
> In the patch 1, I created two sample tests files,
> 2slot_5vcpu_10iter.test and no_dirty_log_protect.test, in the directory
> tools/testing/selftests/kvm/tests/dirty_log_perf_test.
> 
> Contents of those files provide non-default arguments to test, for example,
> 2slot_5vcpu10iter.test has the command:
> 
>   dirty_log_perf_test -x 2 -v 5  -i 10
> 
> One can run these non-default tests as (assuming current directory is
> kvm selftests):
> 
>   python3 runner -d ./tests
> 
> Over the time we will add more of these non-default interesting
> testcases. One can then run:
> 
>   python3 runner -d ./tests ./testcases_default_gen

That's not what I am complaining about. What you call "configuration"
seems to just be "random set of parameters for a random test".

In practice, your runner does not seem configurable at all. You just
treat all possible configurations of a single test as different tests.

My (admittedly very personal) view of what a configuration should be
is "run this single test with these parameters varying in these
ranges, for this long".

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

  reply	other threads:[~2025-10-02 14:41 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-30 16:36 [PATCH v3 0/9] KVM Selftest Runner Vipin Sharma
2025-09-30 16:36 ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 1/9] KVM: selftest: Create KVM selftest runner Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 22:23   ` Vipin Sharma
2025-09-30 22:23     ` Vipin Sharma
2025-10-10  9:47   ` Brendan Jackman
2025-10-10  9:47     ` Brendan Jackman
2025-09-30 16:36 ` [PATCH v3 2/9] KVM: selftests: Provide executables path option to the " Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 3/9] KVM: selftests: Add timeout option in selftests runner Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 4/9] KVM: selftests: Add option to save selftest runner output to a directory Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 5/9] KVM: selftests: Run tests concurrently in KVM selftests runner Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 6/9] KVM: selftests: Add various print flags to KVM selftest runner Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 7/9] KVM: selftests: Print sticky KVM selftests runner status at bottom Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-09-30 16:36 ` [PATCH v3 9/9] KVM: selftests: Provide README.rst " Vipin Sharma
2025-09-30 16:36   ` Vipin Sharma
2025-10-01  8:44   ` Marc Zyngier
2025-10-01  8:44     ` Marc Zyngier
2025-10-01 17:32     ` Vipin Sharma
2025-10-01 17:32       ` Vipin Sharma
2025-10-02 14:41       ` Marc Zyngier [this message]
2025-10-02 14:41         ` Marc Zyngier
2025-10-03  1:02         ` Sean Christopherson
2025-10-03  1:02           ` Sean Christopherson
2025-10-03  6:39         ` Vipin Sharma
2025-10-03  6:39           ` Vipin Sharma
2025-10-10  9:58   ` Brendan Jackman
2025-10-10  9:58     ` Brendan Jackman
2025-10-10 18:14     ` Sean Christopherson
2025-10-10 18:14       ` Sean Christopherson
2025-10-10 19:38       ` Vipin Sharma
2025-10-10 19:38         ` Vipin Sharma

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=86a529z7qh.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=ajones@ventanamicro.com \
    --cc=anup@brainfault.org \
    --cc=atish.patra@linux.dev \
    --cc=borntraeger@linux.ibm.com \
    --cc=chenhuacai@kernel.org \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maobibo@loongson.cn \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vipinsh@google.com \
    --cc=zhaotianrui@loongson.cn \
    /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.