linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Robert Richter <robert.richter@amd.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Stephane Eranian <eranian@google.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] perf, x86: Fix event scheduler for constraints with overlapping counters
Date: Thu, 01 Sep 2011 14:56:46 +0200	[thread overview]
Message-ID: <1314881807.11566.23.camel@twins> (raw)
In-Reply-To: <20110520031855.GA17196@erda.amd.com>

On Fri, 2011-05-20 at 05:18 +0200, Robert Richter wrote:
> On 19.05.11 14:06:50, Ingo Molnar wrote:
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > On Wed, 2011-05-18 at 23:20 +0200, Ingo Molnar wrote:
> > > > * Peter Zijlstra <peterz@infradead.org> wrote:
> > > > 
> > > > > >                         if (c->weight != w)
> > > > > >                                 continue;
> > > > > >  
> > > > > > -                       for_each_set_bit(j, c->idxmsk, X86_PMC_IDX_MAX) {
> > > > > > -                               if (!test_bit(j, used_mask))
> > > > > > +                       /* for each bit in idxmsk starting from idx */
> > > > > > +                       while (idx < X86_PMC_IDX_MAX) {
> > > > > > +                               idx = find_next_bit(c->idxmsk, X86_PMC_IDX_MAX,
> > > > > > +                                                   idx);
> > > > > 
> > > > > I'd be mighty tempted to ignore that 80 column rule here ;-)
> > > > 
> > > > Please put the body of the loop into a helper function, the function is large 
> > > > and there are countless col80 uglinesses in it!
> > > 
> > > I just tried that, its real ugly due to the amount of state you need to
> > > pass around.
> > 
> > Does it help if you put that state into a helper structure?
> 
> Yes, this is what I have in mind too. We could iterate on such a state
> stucture instead of a couple of single variables. Storing and
> restoring the state will then just copying the structure.

Any word on this work? I just noticed we actually need this for Intel
too, the fixed purpose events have overlapping but non-identical
constraint masks.

Now we could optimize the Intel case by always iterating from the top
down, but it won't cure all cases. For example, suppose one counter
(that could be on a FP reg) previously got scheduled on a GP register
and we take the fast-path, in that case we would still end up
under-utilized.



  reply	other threads:[~2011-09-01 12:57 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-16  0:27 [PATCH 0/4] perf, x86: Fixes for v2.6.39 Robert Richter
2011-04-16  0:27 ` [PATCH 1/4] perf, x86: Fix pre-defined cache-misses event for AMD family 15h cpus Robert Richter
2011-04-19 12:03   ` [tip:perf/urgent] " tip-bot for Andre Przywara
2011-04-16  0:27 ` [PATCH 2/4] perf, x86: Fix AMD family 15h FPU event constraints Robert Richter
2011-04-19 12:04   ` [tip:perf/urgent] " tip-bot for Robert Richter
2011-04-16  0:27 ` [PATCH 3/4] perf, x86: Use ALTERNATIVE() to check for X86_FEATURE_PERFCTR_CORE Robert Richter
2011-04-18 20:00   ` Andi Kleen
2011-04-19 10:39     ` Robert Richter
2011-04-19 18:21       ` Andi Kleen
2011-04-19 12:04   ` [tip:perf/core] " tip-bot for Robert Richter
2011-04-16  0:27 ` [PATCH 4/4] perf, x86: Fix event scheduler to solve complex scheduling problems Robert Richter
2011-04-16  8:51   ` Peter Zijlstra
2011-04-16  9:43     ` Ingo Molnar
2011-04-16 10:08       ` Peter Zijlstra
2011-04-16 10:14         ` Ingo Molnar
2011-04-16 10:15           ` Peter Zijlstra
2011-04-16 14:26         ` Valdis.Kletnieks
2011-04-17  8:15     ` Robert Richter
2011-04-17  8:18       ` Ingo Molnar
2011-04-17  8:53         ` Peter Zijlstra
2011-04-17 11:23           ` Robert Richter
2011-04-18  8:17             ` Robert Richter
2011-04-16 15:52   ` Stephane Eranian
2011-04-17  8:44     ` Robert Richter
2011-04-17  9:05       ` Stephane Eranian
2011-04-19 10:26   ` [PATCH v2] perf, x86: Fix event scheduler for constraints with overlapping counters Robert Richter
2011-04-19 11:29     ` Ingo Molnar
2011-04-19 13:55       ` Robert Richter
2011-04-28  9:50         ` Robert Richter
2011-05-18 21:16     ` Peter Zijlstra
2011-05-18 21:20       ` Ingo Molnar
2011-05-18 21:36         ` Peter Zijlstra
2011-05-19 10:49           ` Robert Richter
2011-05-19 18:06           ` Ingo Molnar
2011-05-20  3:18             ` Robert Richter
2011-09-01 12:56               ` Peter Zijlstra [this message]
2011-09-01 14:12                 ` Robert Richter
2011-09-01 16:37                   ` Peter Zijlstra

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=1314881807.11566.23.camel@twins \
    --to=peterz@infradead.org \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=robert.richter@amd.com \
    /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;
as well as URLs for NNTP newsgroup(s).