linux-kernel - Re: [PATCH 4/5] nds32: Fix perf multiple events map to same counter.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181018142905.bn73t7rvtdjr2tbf@lakrids.cambridge.arm.com>
Date:   Thu, 18 Oct 2018 15:29:05 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Nickhu <nickhu@...estech.com>
Cc:     greentime@...estech.com, linux-kernel@...r.kernel.org,
        robh+dt@...nel.org, deanbo422@...il.com, peterz@...radead.org,
        mingo@...hat.com, acme@...nel.org,
        alexander.shishkin@...ux.intel.com, jolsa@...hat.com,
        namhyung@...nel.org, arnd@...db.de, sboyd@...eaurora.org,
        geert@...ux-m68k.org, zong@...estech.com, ebiederm@...ssion.com,
        akpm@...ux-foundation.org, gregkh@...uxfoundation.org,
        pombredanne@...b.com, tglx@...utronix.de,
        kstewart@...uxfoundation.org, devicetree@...r.kernel.org,
        green.hu@...il.com
Subject: Re: [PATCH 4/5] nds32: Fix perf multiple events map to same counter.

On Thu, Oct 18, 2018 at 04:43:16PM +0800, Nickhu wrote:
> When there are multiple events map to the same counter, the counter
> counts inaccurately. This is because each counter only counts one event
> in the same time.
> So when there are multiple events map to same counter, they have to take
> turns in each context.
> 
> There are two solution:
> 1. Print the error message when multiple events map to the same counter.
> But print the error message would let the program hang in loop. The ltp
> (linux test program) would be failed when the program hang in loop.
> 
> 2. Don't print the error message, the ltp would pass. But the user need to
> have the knowledge that don't count the events which map to the same
> counter, or the user will get the inaccurate results.
> 
> We choose method 2 for the solution

This is the correct solution. Perf exposes the active/enabled time in
the perf event, so the user can determine that the event wasn't enabled
all of the time.

This should be folded into the commit adding perf support.

Thanks,
Mark.

> 
> Signed-off-by: Nickhu <nickhu@...estech.com>
> ---
>  arch/nds32/include/asm/pmu.h       |  1 +
>  arch/nds32/kernel/perf_event_cpu.c | 30 ++++++++++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/nds32/include/asm/pmu.h b/arch/nds32/include/asm/pmu.h
> index 3fbbe97c2d42..e75ec34af5f6 100644
> --- a/arch/nds32/include/asm/pmu.h
> +++ b/arch/nds32/include/asm/pmu.h
> @@ -55,6 +55,7 @@ enum { PFMC0, PFMC1, PFMC2, MAX_COUNTERS };
>   */
>  #define NDS32_IDX_CYCLE_COUNTER			0
>  #define NDS32_IDX_COUNTER0			1
> +#define NDS32_IDX_COUNTER1			2
>  #define NDS32_IDX_COUNTER_LAST(cpu_pmu) \
>  	(NDS32_IDX_CYCLE_COUNTER + (cpu_pmu)->num_events - 1)
>  
> diff --git a/arch/nds32/kernel/perf_event_cpu.c b/arch/nds32/kernel/perf_event_cpu.c
> index 7bb4ebb87b5c..e9a0d8bb2bc1 100644
> --- a/arch/nds32/kernel/perf_event_cpu.c
> +++ b/arch/nds32/kernel/perf_event_cpu.c
> @@ -566,16 +566,26 @@ static int nds32_pmu_get_event_idx(struct pmu_hw_events *cpuc,
>  	/*
>  	 * Try to get the counter for correpsonding event
>  	 */
> -	if (!test_and_set_bit(idx, cpuc->used_mask))
> -		return idx;
> -
> -	/*
> -	 * The counter is in use.
> -	 * The system will hang in the loop.
> -	 */
> -	pr_err
> -	("Multiple events map to one counter, the behavior is undefined.\n");
> -	return -EPERM;
> +	if (evtype == SPAV3_0_SEL_TOTAL_CYCLES) {
> +		if (!test_and_set_bit(idx, cpuc->used_mask))
> +			return idx;
> +		if (!test_and_set_bit(NDS32_IDX_COUNTER0, cpuc->used_mask))
> +			return NDS32_IDX_COUNTER0;
> +		if (!test_and_set_bit(NDS32_IDX_COUNTER1, cpuc->used_mask))
> +			return NDS32_IDX_COUNTER1;
> +	} else if (evtype == SPAV3_1_SEL_COMPLETED_INSTRUCTION) {
> +		if (!test_and_set_bit(idx, cpuc->used_mask))
> +			return idx;
> +		else if (!test_and_set_bit(NDS32_IDX_COUNTER1, cpuc->used_mask))
> +			return NDS32_IDX_COUNTER1;
> +		else if (!test_and_set_bit
> +			 (NDS32_IDX_CYCLE_COUNTER, cpuc->used_mask))
> +			return NDS32_IDX_CYCLE_COUNTER;
> +	} else {
> +		if (!test_and_set_bit(idx, cpuc->used_mask))
> +			return idx;
> +	}
> +	return -EAGAIN;
>  }
>  
>  static void nds32_pmu_start(struct nds32_pmu *cpu_pmu)
> -- 
> 2.17.0
>