Skip to contentSkip to conversation

KVM: x86/vPMU: refine kvm_pmu err msg when event creation failed

PATCH Superseded Like Xu 2 participants 3 messages Jul 18, 2019

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Like Xu

author
#0 lore raw

PATCH 1/1 KVM: x86/vPMU: refine kvm_pmu err msg when event creation failed

If a perf_event creation fails due to any reason of the host perf subsystem, it has no chance to log the corresponding event for guest which may cause abnormal sampling data in guest result. In debug mode, this message helps to understand the state of vPMC and we should not limit the number of occurrences.

---
 arch/x86/kvm/pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kvm/pmu.c +2 -2
index aa5a2597305a..259ada8d6db6 100644
@@ -131,8 +131,8 @@ static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type,
 						 intr ? kvm_perf_overflow_intr :
 						 kvm_perf_overflow, pmc);
 	if (IS_ERR(event)) {
-		printk_once("kvm_pmu: event creation failed %ld\n",
-			    PTR_ERR(event));
+		pr_debug("kvm_pmu: event creation failed %ld\n for pmc->idx = %d",
+			    PTR_ERR(event), pmc->idx);
 		return;
 	}
 
Signature · 1 line
2.21.0
#1 lore raw
Quoted 5 lines · Like Xu

If a perf_event creation fails due to any reason of the host perf subsystem, it has no chance to log the corresponding event for guest which may cause abnormal sampling data in guest result. In debug mode, this message helps to understand the state of vPMC and we should not limit the number of occurrences.

[]

arch/x86/kvm/pmu.c

Quoted 1 line
arch/x86/kvm/pmu.c +0 -0

[]

arch/x86/kvm/pmu.c static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type, · line 135

Quoted 8 lines
arch/x86/kvm/pmu.c +2 -2
@@ -131,8 +131,8 @@ static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type,
 						 intr ? kvm_perf_overflow_intr :
 						 kvm_perf_overflow, pmc);
 	if (IS_ERR(event)) {
-		printk_once("kvm_pmu: event creation failed %ld\n",
-			    PTR_ERR(event));
+		pr_debug("kvm_pmu: event creation failed %ld\n for pmc->idx = %d",
+			    PTR_ERR(event), pmc->idx);

Perhaps this was written as printk_once to avoid
spamming the log with repeated messages.

Maybe this should use pr_debug_ratelimited.
(and it should also have a \n termination like:)

		pr_debug_ratelimited("kvm_pmu: event creation failed %ld for pmc->idx = %d\n",
				     PTR_ERR(event), pmc->idx);

Perhaps Avi Kivity remembers why he wrote it this way.
https://lore.kernel.org/kvm/[email protected]/

author
#2 lore raw

Hi Joe,

Quoted 9 lines · Joe Perches
Quoted 5 lines · Like Xu

If a perf_event creation fails due to any reason of the host perf subsystem, it has no chance to log the corresponding event for guest which may cause abnormal sampling data in guest result. In debug mode, this message helps to understand the state of vPMC and we should not limit the number of occurrences.

[]

arch/x86/kvm/pmu.c

Quoted 1 line
arch/x86/kvm/pmu.c +0 -0

[]

Quoted 11 lines

arch/x86/kvm/pmu.c static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type, · line 135

Quoted 8 lines
arch/x86/kvm/pmu.c +2 -2
@@ -131,8 +131,8 @@ static void pmc_reprogram_counter(struct kvm_pmc *pmc, u32 type,
  						 intr ? kvm_perf_overflow_intr :
  						 kvm_perf_overflow, pmc);
  	if (IS_ERR(event)) {
-		printk_once("kvm_pmu: event creation failed %ld\n",
-			    PTR_ERR(event));
+		pr_debug("kvm_pmu: event creation failed %ld\n for pmc->idx = %d",
+			    PTR_ERR(event), pmc->idx);

Perhaps this was written as printk_once to avoid
spamming the log with repeated messages.

The spamming case in practice from this messages is very rare but it's logically possible.

Quoted 3 lines

Maybe this should use pr_debug_ratelimited.
(and it should also have a \n termination like:)

Thanks and it's my mistake, agree on '\n' usage.

Quoted 7 lines
		pr_debug_ratelimited("kvm_pmu: event creation failed %ld for pmc->idx = %d\n",
				     PTR_ERR(event), pmc->idx);

Perhaps Avi Kivity remembers why he wrote it this way.
https://lore.kernel.org/kvm/[email protected] >>