diff mbox series

[2/2] selftests: kvm: Reduce verbosity of "Random seed" messages

Message ID 20240619182128.4131355-3-dapeng1.mi@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series KVM vPMU code refine | expand

Commit Message

Mi, Dapeng June 19, 2024, 6:21 p.m. UTC
Huge number of "Random seed:" messages are printed when running
pmu_counters_test. It leads to the regular test output is totally
flooded by these over-verbose messages.

Downgrade "Random seed" message printing level to debug and prevent it
to be printed in normal case.

Reported-by: Yi Lai <yi1.lai@intel.com>
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sean Christopherson June 20, 2024, 6:13 p.m. UTC | #1
On Thu, Jun 20, 2024, Dapeng Mi wrote:
> Huge number of "Random seed:" messages are printed when running
> pmu_counters_test. It leads to the regular test output is totally
> flooded by these over-verbose messages.
> 
> Downgrade "Random seed" message printing level to debug and prevent it
> to be printed in normal case.

I completely agree this is annoying, but the whole point of printing the seed is
so that the seed is automatically captured if a test fails, e.g. so that the
failure can be reproduced if it is dependent on some random decision.

Rather than simply hiding the message, what if print the seed if and only if it
changes?

--
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 20 Jun 2024 10:29:53 -0700
Subject: [PATCH] KVM: selftests: Print the seed for the guest pRNG iff it has
 changed

Print the guest's random seed during VM creation if and only if the seed
has changed since the seed was last printed.  The vast majority of tests,
if not all tests at this point, set the seed during test initialization
and never change the seed, i.e. printing it every time a VM is created is
useless noise.

Snapshot and print the seed during early selftest init to play nice with
tests that use the kselftests harness, at the cost of printing an unused
seed for tests that change the seed during test-specific initialization,
e.g. dirty_log_perf_test.  The kselftests harness runs each testcase in a
separate process that is forked from the original process before creating
each testcase's VM, i.e. waiting until first VM creation will result in
the seed being printed by each testcase despite it never changing.  And
long term, the hope/goal is that setting the seed will be handled by the
core framework, i.e. that the dirty_log_perf_test wart will naturally go
away.

Reported-by: Yi Lai <yi1.lai@intel.com>
Reported-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index ad00e4761886..56b170b725b3 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -21,6 +21,7 @@
 
 uint32_t guest_random_seed;
 struct guest_random_state guest_rng;
+static uint32_t last_guest_seed;
 
 static int vcpu_mmap_sz(void);
 
@@ -434,7 +435,10 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
 	slot0 = memslot2region(vm, 0);
 	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
 
-	pr_info("Random seed: 0x%x\n", guest_random_seed);
+	if (guest_random_seed != last_guest_seed) {
+		pr_info("Random seed: 0x%x\n", guest_random_seed);
+		last_guest_seed = guest_random_seed;
+	}
 	guest_rng = new_guest_random_state(guest_random_seed);
 	sync_global_to_guest(vm, guest_rng);
 
@@ -2319,7 +2323,8 @@ void __attribute((constructor)) kvm_selftest_init(void)
 	/* Tell stdout not to buffer its content. */
 	setbuf(stdout, NULL);
 
-	guest_random_seed = random();
+	guest_random_seed = last_guest_seed = random();
+	pr_info("Random seed: 0x%x\n", guest_random_seed);
 
 	kvm_selftest_arch_init();
 }

base-commit: c81b138d5075c6f5ba3419ac1d2a2e7047719c14
--
Mi, Dapeng June 21, 2024, 2:51 a.m. UTC | #2
On 6/21/2024 2:13 AM, Sean Christopherson wrote:
> On Thu, Jun 20, 2024, Dapeng Mi wrote:
>> Huge number of "Random seed:" messages are printed when running
>> pmu_counters_test. It leads to the regular test output is totally
>> flooded by these over-verbose messages.
>>
>> Downgrade "Random seed" message printing level to debug and prevent it
>> to be printed in normal case.
> I completely agree this is annoying, but the whole point of printing the seed is
> so that the seed is automatically captured if a test fails, e.g. so that the
> failure can be reproduced if it is dependent on some random decision.
>
> Rather than simply hiding the message, what if print the seed if and only if it
> changes?

Yeah, it's indeed better.

>
> --
> From: Sean Christopherson <seanjc@google.com>
> Date: Thu, 20 Jun 2024 10:29:53 -0700
> Subject: [PATCH] KVM: selftests: Print the seed for the guest pRNG iff it has
>  changed

s/iff/if/


>
> Print the guest's random seed during VM creation if and only if the seed
> has changed since the seed was last printed.  The vast majority of tests,
> if not all tests at this point, set the seed during test initialization
> and never change the seed, i.e. printing it every time a VM is created is
> useless noise.
>
> Snapshot and print the seed during early selftest init to play nice with
> tests that use the kselftests harness, at the cost of printing an unused
> seed for tests that change the seed during test-specific initialization,
> e.g. dirty_log_perf_test.  The kselftests harness runs each testcase in a
> separate process that is forked from the original process before creating
> each testcase's VM, i.e. waiting until first VM creation will result in
> the seed being printed by each testcase despite it never changing.  And
> long term, the hope/goal is that setting the seed will be handled by the
> core framework, i.e. that the dirty_log_perf_test wart will naturally go
> away.
>
> Reported-by: Yi Lai <yi1.lai@intel.com>
> Reported-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index ad00e4761886..56b170b725b3 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -21,6 +21,7 @@
>  
>  uint32_t guest_random_seed;
>  struct guest_random_state guest_rng;
> +static uint32_t last_guest_seed;
>  
>  static int vcpu_mmap_sz(void);
>  
> @@ -434,7 +435,10 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>  	slot0 = memslot2region(vm, 0);
>  	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
>  
> -	pr_info("Random seed: 0x%x\n", guest_random_seed);
> +	if (guest_random_seed != last_guest_seed) {
> +		pr_info("Random seed: 0x%x\n", guest_random_seed);
> +		last_guest_seed = guest_random_seed;
> +	}
>  	guest_rng = new_guest_random_state(guest_random_seed);
>  	sync_global_to_guest(vm, guest_rng);
>  
> @@ -2319,7 +2323,8 @@ void __attribute((constructor)) kvm_selftest_init(void)
>  	/* Tell stdout not to buffer its content. */
>  	setbuf(stdout, NULL);
>  
> -	guest_random_seed = random();
> +	guest_random_seed = last_guest_seed = random();
> +	pr_info("Random seed: 0x%x\n", guest_random_seed);
>  
>  	kvm_selftest_arch_init();
>  }
>
> base-commit: c81b138d5075c6f5ba3419ac1d2a2e7047719c14
Sean Christopherson June 21, 2024, 1:29 p.m. UTC | #3
On Fri, Jun 21, 2024, Dapeng Mi wrote:
> > --
> > From: Sean Christopherson <seanjc@google.com>
> > Date: Thu, 20 Jun 2024 10:29:53 -0700
> > Subject: [PATCH] KVM: selftests: Print the seed for the guest pRNG iff it has
> >  changed
> 
> s/iff/if/

"iff" is shorthand for "if and only if".  I try to write out the full "if and only
if" when possible, but use "iff" in shortlogs when I want to squeeze in more words.
Mi, Dapeng June 26, 2024, 1:57 a.m. UTC | #4
On 6/21/2024 9:29 PM, Sean Christopherson wrote:
> On Fri, Jun 21, 2024, Dapeng Mi wrote:
>>> --
>>> From: Sean Christopherson <seanjc@google.com>
>>> Date: Thu, 20 Jun 2024 10:29:53 -0700
>>> Subject: [PATCH] KVM: selftests: Print the seed for the guest pRNG iff it has
>>>  changed
>> s/iff/if/
> "iff" is shorthand for "if and only if".  I try to write out the full "if and only
> if" when possible, but use "iff" in shortlogs when I want to squeeze in more words.

Good to know. Thanks.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index ad00e4761886..8568c7d619c3 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -434,7 +434,7 @@  struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
 	slot0 = memslot2region(vm, 0);
 	ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size);
 
-	pr_info("Random seed: 0x%x\n", guest_random_seed);
+	pr_debug("Random seed: 0x%x\n", guest_random_seed);
 	guest_rng = new_guest_random_state(guest_random_seed);
 	sync_global_to_guest(vm, guest_rng);