Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 4 Sep 2018 11:23:12 -0700
From:   Eduardo Valentin <eduval@amazon.com>
To:     Eduardo Valentin <eduval@amazon.com>
CC:     Andi Kleen <ak@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, <x86@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Kan Liang <kan.liang@linux.intel.com>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        "Jia Zhang" <qianyue.zj@alibaba-inc.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCHv2 1/1] perf/x86/intel: make error messages less confusing
Message-ID: <20180904182312.GA11911@u40b0340c692b58f6553c.ant.amazon.com>
References: <20180823150732.11249-1-eduval@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20180823150732.11249-1-eduval@amazon.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello,

On Thu, Aug 23, 2018 at 08:07:32AM -0700, Eduardo Valentin wrote:
> On a system with X86_FEATURE_ARCH_PERFMON disabled
> and with a model not known by family PMU drivers,
> user gets a kernel message log like the following:
> [ 0.100114] Performance Events: unsupported p6 CPU model 85 no PMU driver, software events only.
> 
> The "unsupported .. CPU" part may be confusing for some
> users leading to wrong understanding that the kernel
> does not support the CPU model.
> 
> This patch rewords the messages on the failure path to:
> [ 0.667154] Performance Events: CPU does not support PMU: no PMU driver, software events only.
> 


Gentle ping on this patch, any further comments?

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Eduardo Valentin <eduval@amazon.com>
> Cc: Jia Zhang <qianyue.zj@alibaba-inc.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin <eduval@amazon.com>
> ---
> 
> Changes from V1->V2:
> - As per initial review, the propose messaging was even
> more confusing. Simplified it by only saying that
> the CPU does not support PMU.
> 
> 
>  arch/x86/events/intel/core.c | 15 +++++++++++----
>  arch/x86/events/intel/p4.c   |  5 +----
>  arch/x86/events/intel/p6.c   |  1 -
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 035c37481f57..2ddb97f03f4a 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3889,15 +3889,22 @@ __init int intel_pmu_init(void)
>  	char *name;
>  
>  	if (!cpu_has(&boot_cpu_data, X86_FEATURE_ARCH_PERFMON)) {
> +		int ret = -ENODEV;
> +
>  		switch (boot_cpu_data.x86) {
>  		case 0x6:
> -			return p6_pmu_init();
> +			ret = p6_pmu_init();
> +			break;
>  		case 0xb:
> -			return knc_pmu_init();
> +			ret = knc_pmu_init();
> +			break;
>  		case 0xf:
> -			return p4_pmu_init();
> +			ret = p4_pmu_init();
> +			break;
>  		}
> -		return -ENODEV;
> +		if (ret)
> +			pr_cont("CPU does not support PMU: ");
> +		return ret;
>  	}
>  
>  	/*
> diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
> index d32c0eed38ca..fb5e8576d9ac 100644
> --- a/arch/x86/events/intel/p4.c
> +++ b/arch/x86/events/intel/p4.c
> @@ -1345,11 +1345,8 @@ __init int p4_pmu_init(void)
>  	BUILD_BUG_ON(ARCH_P4_MAX_CCCR > INTEL_PMC_MAX_GENERIC);
>  
>  	rdmsr(MSR_IA32_MISC_ENABLE, low, high);
> -	if (!(low & (1 << 7))) {
> -		pr_cont("unsupported Netburst CPU model %d ",
> -			boot_cpu_data.x86_model);
> +	if (!(low & (1 << 7)))
>  		return -ENODEV;
> -	}
>  
>  	memcpy(hw_cache_event_ids, p4_hw_cache_event_ids,
>  		sizeof(hw_cache_event_ids));
> diff --git a/arch/x86/events/intel/p6.c b/arch/x86/events/intel/p6.c
> index 408879b0c0d4..e8e03e68b22f 100644
> --- a/arch/x86/events/intel/p6.c
> +++ b/arch/x86/events/intel/p6.c
> @@ -269,7 +269,6 @@ __init int p6_pmu_init(void)
>  		break;
>  
>  	default:
> -		pr_cont("unsupported p6 CPU model %d ", boot_cpu_data.x86_model);
>  		return -ENODEV;
>  	}
>  
> -- 
> 2.18.0
> 

-- 
All the best,
Eduardo Valentin