linux-kernel - Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters

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] [day] [month] [year] [list]

Message-ID: <55719DDB.9060507@gmail.com>
Date:	Fri, 05 Jun 2015 15:02:19 +0200
From:	Imre Palik <imrep.amz@...il.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-kernel@...r.kernel.org, "Palik, Imre" <imrep@...zon.de>,
	Anthony Liguori <aliguori@...zon.com>
Subject: Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters

On 06/04/15 15:29, Peter Zijlstra wrote:
> On Thu, Jun 04, 2015 at 02:30:37PM +0200, Imre Palik wrote:
>> The trouble is that the number of fixed counters is not taken into
>> account when scheduling the events, and the cpu model based event
>> constraints will favour fixed counters.  So perf tries to use them.
> 
> Ah! so that is what your hunk below does. Tricky, and without comment
> that.
> 
> ---
> 
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index 87848eb..eaa0b5e 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -647,6 +647,8 @@ static void perf_sched_init(struct perf_sched *sched, struct perf_event **events
>         sched->state.event      = idx;          /* start with min weight */
>         sched->state.weight     = wmin;
>         sched->state.unassigned = num;
> +       sched->state.used[0]    =
> +               ~0UL << (INTEL_PMC_IDX_FIXED + x86_pmu.num_counters_fixed);
>  }
> 
>  static void perf_sched_save_state(struct perf_sched *sched)
> 
> ---
> 
> Please change the FIXED_EVENT constraints init instead; that way
> validate_event() will actually work too, otherwise it thinks it can
> schedule the fixed function only events.
> 
> That is, change the below loop from intel_pmu_init():
> 
> 	if (x86_pmu.event_constraints) {
> 		/*
> 		 * event on fixed counter2 (REF_CYCLES) only works on this
> 		 * counter, so do not extend mask to generic counters
> 		 */
> 		for_each_event_constraint(c, x86_pmu.event_constraints) {
> 			if (c->cmask != FIXED_EVENT_FLAGS
> 			    || c->idxmsk64 == INTEL_PMC_MSK_FIXED_REF_CYCLES) {
> 				continue;
> 			}
> 
> 			c->idxmsk64 |= (1ULL << x86_pmu.num_counters) - 1;
> 			c->weight += x86_pmu.num_counters;
> 		}
> 	}
> 
> To clear all counters that are not in fact present, that way we keep the
> event constraints correct instead of working around invalid constraints.

Thanks.

I knew there should be a better way ...

Will post a new patch soon.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/