All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Song Liu <songliubraving@fb.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	"acme@kernel.org" <acme@kernel.org>,
	"alexander.shishkin@linux.intel.com" 
	<alexander.shishkin@linux.intel.com>,
	"jolsa@redhat.com" <jolsa@redhat.com>,
	"eranian@google.com" <eranian@google.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"alexey.budankov@linux.intel.com"
	<alexey.budankov@linux.intel.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"megha.dey@intel.com" <megha.dey@intel.com>,
	"frederic@kernel.org" <frederic@kernel.org>
Subject: Re: [RFC][PATCH] perf: Rewrite core context handling
Date: Tue, 16 Oct 2018 11:50:56 +0200	[thread overview]
Message-ID: <20181016095056.GE4030@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <E4F81A5E-C371-429D-AFEC-6F4B32971F09@fb.com>

On Sat, Oct 13, 2018 at 08:31:37AM +0000, Song Liu wrote:

> The only suggestion I have right now is on which struct owns which
> data:
> 
> 1. perf_cpu_context owns two perf_event_context: ctx and *task_ctx. 
>    This is the same as right now. 

> 2. perf_event_context owns multiple perf_event_pmu_context: 
>    One perf_event_pmu_context for software groups;
>    One perf_event_pmu_context for each hardware PMU.

It does now already, right? Through the pmu_ctx_list we can, given an
perf_event_context, find all associated perf_event_pmu_context's.

> 3. perf_event_pmu_context owns RB tree of events. Since we don't 
>    need rotation across multiple hardware PMUs, the rotation is 
>    within same perf_event_pmu_context.  

By keeping the RB trees in perf_event_context, we get bigger trees,
which is more efficient (log(n+m) < log(n) + log(m))

Also, specifically, it means we only need a single merge sort /
iteration to schedule in a full context, instead of (again) doing 'n' of
them.

Also, given a context and a pmu, it is cheaper for finding the relevant
events; this is needed for big.little for instance. Something the
proposed patch doesn't fully flesh out.

> 4. perf_cpu_context owns multiple perf_cpu_pmu_context:
>    One perf_cpu_pmu_context for each hardware PMU.

What would we need that relation for?

>    perf_cpu_pmu_context is tot needed for software only groups(?).

Yes, that is a very good question; it mostly centers around what we want
to do with perf_event_attr::exclusive for software events -- which is
currently dodgy at best.

Also, allocating the structure and keeping it around is probably less
code than explicitly not doing it.

> 5. perf_cpu_pmu_context has two pointers of perf_event_pmu_context.

Instead of embedding the thing? Yeah, not sure. Either way around we'd
not want to free the CPU perf_event_pmu_context that is associated with
the perf_cpu_pmu_context, and embedding it saves a pointer chase.

Not sure it actually makes a lot of difference either way around.

  reply	other threads:[~2018-10-16  9:51 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10 10:45 [RFC][PATCH] perf: Rewrite core context handling Peter Zijlstra
2018-10-11  7:50 ` Song Liu
2018-10-11  9:29   ` Peter Zijlstra
2018-10-11 22:37     ` Song Liu
2018-10-12  9:50       ` Peter Zijlstra
2018-10-12 14:25         ` Peter Zijlstra
2018-10-13  8:31         ` Song Liu
2018-10-16  9:50           ` Peter Zijlstra [this message]
2018-10-16 16:34             ` Song Liu
2018-10-16 18:10               ` Peter Zijlstra
2018-10-16 18:24                 ` Song Liu
2018-10-12  7:04     ` Alexey Budankov
2018-10-12 11:54       ` Peter Zijlstra
2018-10-15  7:26 ` Alexey Budankov
2018-10-15  8:34   ` Peter Zijlstra
2018-10-15  8:53     ` Peter Zijlstra
2018-10-15 17:29     ` Alexey Budankov
2018-10-15 18:31       ` Stephane Eranian
2018-10-16  6:39         ` Alexey Budankov
2018-10-16  9:32         ` Peter Zijlstra
2018-10-15 22:09     ` Song Liu
2018-10-16 18:28       ` Song Liu
2018-10-17 11:06         ` Peter Zijlstra
2018-10-17 16:43           ` Song Liu
2018-10-17 17:19             ` Peter Zijlstra
2018-10-17 18:33               ` Peter Zijlstra
2018-10-17 18:57                 ` Song Liu
2018-10-16 16:26 ` Mark Rutland
2018-10-16 18:07   ` Peter Zijlstra
2018-10-17  8:57 ` Alexey Budankov
2018-10-17 15:01   ` Alexander Shishkin
2018-10-17 15:58     ` Alexey Budankov
2018-10-17 16:30   ` Peter Zijlstra
2018-10-18  7:05     ` Alexey Budankov
2018-10-22 13:26 ` Alexander Shishkin
2018-10-23  6:13 ` Song Liu
2018-10-23  6:55   ` Peter Zijlstra
2019-05-15 11:17 ` Alexander Shishkin

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=20181016095056.GE4030@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=alexey.budankov@linux.intel.com \
    --cc=eranian@google.com \
    --cc=frederic@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=megha.dey@intel.com \
    --cc=mingo@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.