Messages in this thread | | | From | Bandan Das <> | Subject | Re: [PATCH] perf/x86: descriptive failure messages for PMU init | Date | Mon, 15 Apr 2019 08:42:04 -0400 |
| |
Hi Peter,
Peter Zijlstra <peterz@infradead.org> writes:
> On Fri, Apr 12, 2019 at 03:09:17PM -0400, Bandan Das wrote: >> >> There's a default warning message that gets printed, however, >> there are various failure conditions: >> - a msr read can fail >> - a msr write can fail >> - a msr has an unexpected value >> - all msrs have unexpected values (disable PMU) >> >> Also, commit commit 005bd0077a79 ("perf/x86: Modify error message in >> virtualized environment") completely removed printing the msr in >> question but these messages could be helpful for debugging vPMUs as >> well. Add them back and change them to pr_debugs, this keeps the >> behavior the same for baremetal. >> >> Lastly, use %llx to silence checkpatch > > Yuck... if you're debugging a hypervisor, you can bloody well run your > own kernel with additional print slattered around. > > The whole make an exception for virt bullshit was already pushing it, > this is just insane. >
The only virt specific parts are the pr_debugs which I can remove and replace with unconditional pr_err()s as suggested by Jiri. Is that ok ?
Bandan
>> @@ -266,12 +282,30 @@ static bool check_hw_exists(void) >> return true; >> >> msr_fail: >> - if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { >> + if (virt) >> pr_cont("PMU not available due to virtualization, using software events only.\n"); >> - } else { >> - pr_cont("Broken PMU hardware detected, using software events only.\n"); >> - pr_err("Failed to access perfctr msr (MSR %x is %Lx)\n", >> - reg, val_new); >> + switch (status) { >> + case READ_FAIL: >> + if (virt) >> + pr_debug("Failed to read perfctr msr (MSR %x)\n", reg); >> + else >> + pr_err("Failed to read perfctr msr (MSR %x)\n", reg); >> + break; >> + case WRITE_FAIL: >> + if (virt) >> + pr_debug("Failed to write perfctr msr (MSR %x, wrote: %llx, read: %llx)\n", >> + reg, val, val_new); >> + else >> + pr_err("Failed to write perfctr msr (MSR %x, wrote: %llx, read: %llx)\n", >> + reg, val, val_new); >> + break; >> + case PMU_FAIL: >> + /* fall through for default message */ >> + default: >> + if (virt) >> + pr_debug("Broken PMU hardware detected, using software events only.\n"); >> + else >> + pr_cont("Broken PMU hardware detected, using software events only.\n"); >> }
| |