public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Imre Palik <imrep.amz@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Paul Mackerras <paulus@samba.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-kernel@vger.kernel.org, "Palik, Imre" <imrep@amazon.de>,
	Anthony Liguori <aliguori@amazon.com>
Subject: Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters
Date: Thu, 4 Jun 2015 15:29:55 +0200	[thread overview]
Message-ID: <20150604132955.GJ3644@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <557044ED.8060508@gmail.com>

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.

  reply	other threads:[~2015-06-04 13:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-03  8:03 [RFC PATCH] perf: honoring cpuid for number of fixed counters Imre Palik
2015-06-03  8:36 ` Peter Zijlstra
2015-06-04 10:35   ` Imre Palik
2015-06-04 11:49     ` Peter Zijlstra
2015-06-04 12:30       ` Imre Palik
2015-06-04 13:29         ` Peter Zijlstra [this message]
2015-06-05 13:02           ` Imre Palik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150604132955.GJ3644@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=aliguori@amazon.com \
    --cc=hpa@zytor.com \
    --cc=imrep.amz@gmail.com \
    --cc=imrep@amazon.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox