All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@redhat.com>,
	Stephane Eranian <eranian@google.com>,
	mathieu.poirier@linaro.org, Pawel Moll <pawel.moll@arm.com>
Subject: Re: [RFC PATCH] perf: Add PERF_RECORD_SWITCH to indicate context switches
Date: Fri, 12 Jun 2015 14:12:11 +0300	[thread overview]
Message-ID: <557ABE8B.1020705@intel.com> (raw)
In-Reply-To: <20150611141548.GW19282@twins.programming.kicks-ass.net>

On 11/06/15 17:15, Peter Zijlstra wrote:
> On Tue, Jun 09, 2015 at 05:21:10PM +0300, Adrian Hunter wrote:
>> Tracepoints are no good at all for non-privileged users
>> because they need either CAP_SYS_ADMIN or
>> /proc/sys/kernel/perf_event_paranoid <= -1.
>>
>> On the other hand, kernel software events need either
>> CAP_SYS_ADMIN or /proc/sys/kernel/perf_event_paranoid <= 1.
> 
> So while I think it makes sense to allow some tracepoint outside of that
> priv level, IOW have a per tracepoint priv level filter thingy, I don't
> think sched_switch() is one of those because it explicitly exposes
> timing information on other tasks.
> 
>> This new PERF_RECORD_SWITCH event does not have those problems
>> and it also has a couple of other small advantages. It is
>> easier to use because it is an auxiliary event (like mmap,
>> comm and task events) which can be enabled by setting a single
>> bit. It is smaller than sched:sched_switch and easier to parse.
> 
> Right, so the one wee problem I have is that this only provides sched_in
> data, I imagine people might be interested in sched_out as well.

That is not a problem although it would be interesting to know the use-case.
To me it seemed unreasonable to expect to analyze scheduler behaviour
without admin-level privileges since it is inherently an administrative
activity.

> 
> Typically the switch even provides prev and next and thereby is
> complete, but since we're limiting it to the one specific task, we'll
> not have the sched_out data.

That makes sense for completeness, but as I wrote, it would be interesting
to know what someone might actually use that for.

> 
>> @@ -812,6 +813,18 @@ enum perf_event_type {
>>  	 */
>>  	PERF_RECORD_ITRACE_START		= 12,
>>  
>> +	/*
>> +	 *
>> +	 *
>> +	 * struct {
>> +	 *	struct perf_event_header	header;
>> +	 *	u32				pid, tid;
>> +	 *	u64				time;
> 
> all 3 are already part of sample_id.

You have to decide whether you expect to be able to use an event without
sample_id. MMAP and MMAP2 both have pid, tid which are in sample_id, LOST
has id, EXIT and FORK have time, all of the THROTTLE/UNTHROTTLE members are
in sample_id etc. So it currently looks like we expect to be able to use an
event without requiring sample_id.

It doesn't affect my case either way because perf tools always sets
sample_id_all if it can.

> 
>> +	 * 	struct sample_id		sample_id;
>> +	 * };
>> + 	 */
>> +	PERF_RECORD_SWITCH			= 13,
> 
> 


  parent reply	other threads:[~2015-06-12 11:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-09 14:21 [RFC PATCH] perf: Add PERF_RECORD_SWITCH to indicate context switches Adrian Hunter
2015-06-11 14:15 ` Peter Zijlstra
2015-06-11 16:34   ` Andi Kleen
2015-06-11 16:54     ` Peter Zijlstra
2015-06-12  0:47   ` David Ahern
2015-06-12 10:34     ` Adrian Hunter
2015-06-12 14:21       ` David Ahern
2015-06-12 16:13         ` Adrian Hunter
2015-06-12 11:12   ` Adrian Hunter [this message]
2015-06-12 12:09     ` Peter Zijlstra
2015-06-12 12:36       ` Arnaldo Carvalho de Melo
2015-06-12 13:15         ` Adrian Hunter
2015-06-12 13:28           ` Pawel Moll
2015-06-12 13:52             ` Pawel Moll
2015-06-12 14:30             ` David Ahern
2015-06-12 14:29     ` David Ahern

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=557ABE8B.1020705@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mingo@kernel.org \
    --cc=pawel.moll@arm.com \
    --cc=peterz@infradead.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 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.