All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf: add container identifier entry in perf sample data
@ 2016-08-25 11:57 Hari Bathini
  2016-08-25 11:58 ` [PATCH 2/2] perf tool: add container identifier entry related changes Hari Bathini
  2016-08-25 13:01 ` [PATCH 1/2] perf: add container identifier entry in perf sample data Peter Zijlstra
  0 siblings, 2 replies; 8+ messages in thread
From: Hari Bathini @ 2016-08-25 11:57 UTC (permalink / raw)
  To: ast, peterz, lkml, acme, alexander.shishkin, mingo
  Cc: sargun, Aravinda Prasad, Naveen N. Rao,
	Ananth N Mavinakayanahalli, daniel

Currently, there is no mechanism to filter events based on containers.
perf -g can be used, but it will not filter events for the containers
created after perf is invoked, making it difficult to assess/analyze
performance issues of multiple containers at once. This limitation can
be overcome, if there is a standard kernel identifier for containers.

This patch introduces a container identifier entry field in perf sample
data to identify or distinguish sample data of different containers. It
uses the cgroup namespace inode number of a given task as it's container
identifier (cid). Alternatively, inode number of pid namespace can also
be used as cid. This patch assumes each container is created with it's
own cgroup namespace.

Suggested-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 include/linux/perf_event.h      |    4 ++++
 include/uapi/linux/perf_event.h |    3 ++-
 kernel/events/core.c            |   15 +++++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8ed43261..d43bbf2 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -904,6 +904,10 @@ struct perf_sample_data {
 
 	struct perf_regs		regs_intr;
 	u64				stack_user_size;
+	struct {
+		u32	cid;
+		u32	reserved;
+	}				cid_entry;
 } ____cacheline_aligned;
 
 /* default value for data source */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index c66a485..fb4f902 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_CID				= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index a19550d..b5d774c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5716,6 +5716,9 @@ void perf_output_sample(struct perf_output_handle *handle,
 		}
 	}
 
+	if (sample_type & PERF_SAMPLE_CID)
+		perf_output_put(handle, data->cid_entry);
+
 	if (!event->attr.watermark) {
 		int wakeup_events = event->attr.wakeup_events;
 
@@ -5849,6 +5852,18 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 		header->size += size;
 	}
+
+	if (sample_type & PERF_SAMPLE_CID) {
+		int size = sizeof(u64);
+
+		/*
+		 * Container identifier for a given task.
+		 * Using cgroup namespace inode number for this.
+		 */
+		data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
+		data->cid_entry.reserved = 0;
+		header->size += size;
+	}
 }
 
 static void __always_inline

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] perf tool: add container identifier entry related changes
  2016-08-25 11:57 [PATCH 1/2] perf: add container identifier entry in perf sample data Hari Bathini
@ 2016-08-25 11:58 ` Hari Bathini
  2016-08-25 13:04   ` Peter Zijlstra
  2016-08-25 13:01 ` [PATCH 1/2] perf: add container identifier entry in perf sample data Peter Zijlstra
  1 sibling, 1 reply; 8+ messages in thread
From: Hari Bathini @ 2016-08-25 11:58 UTC (permalink / raw)
  To: ast, peterz, lkml, acme, alexander.shishkin, mingo
  Cc: sargun, Aravinda Prasad, Naveen N. Rao,
	Ananth N Mavinakayanahalli, daniel

With the introduction of container identifier entry in sample data,
perf sample data can now be analyzed with regard to containers. This
patch adds cid entry related support in perf tool.

Shown below is the output of perf report, sorted based on cid, on a
system that was running three containers at the time of perf record
and clearly showing one of the containers' considerable use of kernel
memory in comparison with others:

	$ perf report -s cid -n --stdio
	#
	# Total Lost Samples: 0
	#
	# Samples: 2K of event 'kmem:kmalloc'
	# Event count (approx.): 2171
	#
	# Overhead       Samples  Container ID
	# ........  ............  .............
	#
	    91.20%          1980  4026532048
	     3.55%            77  4026532105
	     2.67%            58  4026532162
	     2.58%            56  4026531835

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 tools/include/uapi/linux/perf_event.h    |    3 ++-
 tools/perf/Documentation/perf-report.txt |    3 ++-
 tools/perf/Documentation/perf-script.txt |    4 ++--
 tools/perf/builtin-script.c              |   14 +++++++++++-
 tools/perf/util/event.h                  |    1 +
 tools/perf/util/evsel.c                  |   34 ++++++++++++++++++++++++++++++
 tools/perf/util/hist.c                   |    2 ++
 tools/perf/util/hist.h                   |    1 +
 tools/perf/util/session.c                |    3 +++
 tools/perf/util/sort.c                   |   22 +++++++++++++++++++
 tools/perf/util/sort.h                   |    2 ++
 11 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index c66a485..fb4f902 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -139,8 +139,9 @@ enum perf_event_sample_format {
 	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
 	PERF_SAMPLE_TRANSACTION			= 1U << 17,
 	PERF_SAMPLE_REGS_INTR			= 1U << 18,
+	PERF_SAMPLE_CID				= 1U << 19,
 
-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
 };
 
 /*
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 2d17462..b081aef 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -68,12 +68,13 @@ OPTIONS
 --sort=::
 	Sort histogram entries by given key(s) - multiple keys can be specified
 	in CSV format.  Following sort keys are available:
-	pid, comm, dso, symbol, parent, cpu, socket, srcline, weight, local_weight.
+	pid, comm, cid, dso, symbol, parent, cpu, socket, srcline, weight, local_weight.
 
 	Each key has following meaning:
 
 	- comm: command (name) of the task which can be read via /proc/<pid>/comm
 	- pid: command and tid of the task
+	- cid: ccontainer id of the task
 	- dso: name of library or module executed at the time of sample
 	- symbol: name of function executed at the time of sample
 	- parent: name of function matched to the parent regex filter. Unmatched
diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 1f6c705..3952783 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -115,8 +115,8 @@ OPTIONS
 -F::
 --fields::
         Comma separated list of fields to print. Options are:
-        comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff,
-	srcline, period, iregs, brstack, brstacksym, flags.
+        comm, tid, pid, cid, time, cpu, event, trace, ip, sym, dso, addr,
+        symoff, srcline, period, iregs, brstack, brstacksym, flags.
         Field list can be prepended with the type, trace, sw or hw,
         to indicate to which event type the field list applies.
         e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 971ff91..fe35dec 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -65,6 +65,7 @@ enum perf_output_field {
 	PERF_OUTPUT_WEIGHT	    = 1U << 18,
 	PERF_OUTPUT_BPF_OUTPUT	    = 1U << 19,
 	PERF_OUTPUT_CALLINDENT	    = 1U << 20,
+	PERF_OUTPUT_CID             = 1U << 21,
 };
 
 struct output_option {
@@ -92,6 +93,7 @@ struct output_option {
 	{.str = "weight",   .field = PERF_OUTPUT_WEIGHT},
 	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
 	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
+	{.str = "cid",   .field = PERF_OUTPUT_CID},
 };
 
 /* default set to maintain compatibility with current format */
@@ -312,6 +314,11 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel,
 					PERF_OUTPUT_IREGS))
 		return -EINVAL;
 
+	if (PRINT_FIELD(CID) &&
+		perf_evsel__check_stype(evsel, PERF_SAMPLE_CID, "CID",
+					PERF_OUTPUT_CID))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -909,6 +916,9 @@ static void process_event(struct perf_script *script,
 	if (perf_evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
 		print_sample_bpf_output(sample);
 
+	if (PRINT_FIELD(CID))
+		printf("%10u ", sample->cid);
+
 	printf("\n");
 }
 
@@ -2114,8 +2124,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_CALLBACK('F', "fields", NULL, "str",
 		     "comma separated output fields prepend with 'type:'. "
 		     "Valid types: hw,sw,trace,raw. "
-		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
-		     "addr,symoff,period,iregs,brstack,brstacksym,flags,"
+		     "Fields: comm,tid,pid,cid,time,cpu,event,trace,ip,sym,"
+		     "dso,addr,symoff,period,iregs,brstack,brstacksym,flags,"
 		     "callindent", parse_output_fields),
 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
 		    "system-wide collection from all CPUs"),
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 8d363d5..c35b1c5 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -191,6 +191,7 @@ struct perf_sample {
 	u32 raw_size;
 	u64 data_src;
 	u32 flags;
+	u32 cid;
 	u16 insn_len;
 	u8  cpumode;
 	void *raw_data;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d9b80ef..d35fbd3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -929,6 +929,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
 	if (opts->sample_transaction)
 		perf_evsel__set_sample_bit(evsel, TRANSACTION);
 
+	perf_evsel__set_sample_bit(evsel, CID);
+
 	if (opts->running_time) {
 		evsel->attr.read_format |=
 			PERF_FORMAT_TOTAL_TIME_ENABLED |
@@ -1973,6 +1975,20 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 		}
 	}
 
+	data->cid = 0;
+	if (type & PERF_SAMPLE_CID) {
+		u.val64 = *array;
+
+		if (swapped) {
+			/* undo swap of u64, then swap on individual u32s */
+			u.val64 = bswap_64(u.val64);
+			u.val32[0] = bswap_32(u.val32[0]);
+		}
+
+		data->cid = u.val32[0];
+		array++;
+	}
+
 	return 0;
 }
 
@@ -2078,6 +2094,9 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
 		}
 	}
 
+	if (type & PERF_SAMPLE_CID)
+		result += sizeof(u64);
+
 	return result;
 }
 
@@ -2267,6 +2286,21 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type,
 		}
 	}
 
+	if (type & PERF_SAMPLE_CID) {
+		u.val32[0] = sample->cid;
+
+		if (swapped) {
+			/*
+			 * Inverse of what is done in perf_evsel__parse_sample
+			 */
+			u.val32[0] = bswap_32(u.val32[0]);
+			u.val64 = bswap_64(u.val64);
+		}
+
+		*array = u.val64;
+		array++;
+	}
+
 	return 0;
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index de15dbc..93d5054 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -168,6 +168,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
 	}
 
+	hists__new_col_len(hists, HISTC_CID, 10);
 	hists__new_col_len(hists, HISTC_CPU, 3);
 	hists__new_col_len(hists, HISTC_SOCKET, 6);
 	hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
@@ -591,6 +592,7 @@ __hists__add_entry(struct hists *hists,
 		.hists	= hists,
 		.branch_info = bi,
 		.mem_info = mi,
+		.cid = sample->cid,
 		.transaction = sample->transaction,
 		.raw_data = sample->raw_data,
 		.raw_size = sample->raw_size,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 0a1edf1..1ad1d91 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -28,6 +28,7 @@ enum hist_column {
 	HISTC_SYMBOL,
 	HISTC_DSO,
 	HISTC_THREAD,
+	HISTC_CID,
 	HISTC_COMM,
 	HISTC_PARENT,
 	HISTC_CPU,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 5d61242..d40f9c7 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1108,6 +1108,9 @@ static void dump_sample(struct perf_evsel *evsel, union perf_event *event,
 
 	if (sample_type & PERF_SAMPLE_READ)
 		sample_read__printf(sample, evsel->attr.read_format);
+
+	if (sample_type & PERF_SAMPLE_CID)
+		printf("... cid: %u\n", sample->cid);
 }
 
 static struct machine *machines__find_for_cpumode(struct machines *machines,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 947d21f..aa2ee43 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1392,6 +1392,27 @@ struct sort_entry sort_transaction = {
 	.se_width_idx	= HISTC_TRANSACTION,
 };
 
+/* --sort cid */
+
+static int64_t
+sort__cid_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	return (int64_t)right->cid - (int64_t)left->cid;
+}
+
+static int hist_entry__cid_snprintf(struct hist_entry *he, char *bf,
+				       size_t size, unsigned int width)
+{
+	return repsep_snprintf(bf, size, "%-*u", width, he->cid);
+}
+
+struct sort_entry sort_cid = {
+	.se_header	= "Container ID ",
+	.se_cmp		= sort__cid_cmp,
+	.se_snprintf	= hist_entry__cid_snprintf,
+	.se_width_idx	= HISTC_CID,
+};
+
 struct sort_dimension {
 	const char		*name;
 	struct sort_entry	*entry;
@@ -1414,6 +1435,7 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
 	DIM(SORT_TRANSACTION, "transaction", sort_transaction),
 	DIM(SORT_TRACE, "trace", sort_trace),
+	DIM(SORT_CID, "cid", sort_cid),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 7ca37ea..eea40e5 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -94,6 +94,7 @@ struct hist_entry {
 	u64			transaction;
 	s32			socket;
 	s32			cpu;
+	u32			cid;
 	u8			cpumode;
 	u8			depth;
 
@@ -210,6 +211,7 @@ enum sort_type {
 	SORT_GLOBAL_WEIGHT,
 	SORT_TRANSACTION,
 	SORT_TRACE,
+	SORT_CID,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] perf: add container identifier entry in perf sample data
  2016-08-25 11:57 [PATCH 1/2] perf: add container identifier entry in perf sample data Hari Bathini
  2016-08-25 11:58 ` [PATCH 2/2] perf tool: add container identifier entry related changes Hari Bathini
@ 2016-08-25 13:01 ` Peter Zijlstra
  2016-08-25 17:20   ` Hari Bathini
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2016-08-25 13:01 UTC (permalink / raw)
  To: Hari Bathini
  Cc: ast, lkml, acme, alexander.shishkin, mingo, sargun,
	Aravinda Prasad, Naveen N. Rao, Ananth N Mavinakayanahalli,
	daniel

On Thu, Aug 25, 2016 at 05:27:54PM +0530, Hari Bathini wrote:

> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index c66a485..fb4f902 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -139,8 +139,9 @@ enum perf_event_sample_format {
>  	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
>  	PERF_SAMPLE_TRANSACTION			= 1U << 17,
>  	PERF_SAMPLE_REGS_INTR			= 1U << 18,
> +	PERF_SAMPLE_CID				= 1U << 19,
>  
> -	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
> +	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
>  };

This forgets to update the comment that goes with PERF_RECORD_SAMPLE.
This patch would also need an update to the manpage:

  http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2

  http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html

> +	if (sample_type & PERF_SAMPLE_CID) {
> +		int size = sizeof(u64);
> +
> +		/*
> +		 * Container identifier for a given task.
> +		 * Using cgroup namespace inode number for this.
> +		 */
> +		data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
> +		data->cid_entry.reserved = 0;
> +		header->size += size;
> +	}
>  }

Does this compile with CONFIG_CGROUP=n ?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] perf tool: add container identifier entry related changes
  2016-08-25 11:58 ` [PATCH 2/2] perf tool: add container identifier entry related changes Hari Bathini
@ 2016-08-25 13:04   ` Peter Zijlstra
  2016-08-25 17:22     ` Hari Bathini
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2016-08-25 13:04 UTC (permalink / raw)
  To: Hari Bathini
  Cc: ast, lkml, acme, alexander.shishkin, mingo, sargun,
	Aravinda Prasad, Naveen N. Rao, Ananth N Mavinakayanahalli,
	daniel

On Thu, Aug 25, 2016 at 05:28:03PM +0530, Hari Bathini wrote:
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d9b80ef..d35fbd3 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -929,6 +929,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
>  	if (opts->sample_transaction)
>  		perf_evsel__set_sample_bit(evsel, TRANSACTION);
>  
> +	perf_evsel__set_sample_bit(evsel, CID);
> +
>  	if (opts->running_time) {
>  		evsel->attr.read_format |=
>  			PERF_FORMAT_TOTAL_TIME_ENABLED |

Does this mean its enabled by default? Why? By growing the default
sample to include everything under the sun we make profiling slower for
everyone.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] perf: add container identifier entry in perf sample data
  2016-08-25 13:01 ` [PATCH 1/2] perf: add container identifier entry in perf sample data Peter Zijlstra
@ 2016-08-25 17:20   ` Hari Bathini
  2016-08-25 17:31     ` Peter Zijlstra
  2016-08-26  4:05     ` Ananth N Mavinakayanahalli
  0 siblings, 2 replies; 8+ messages in thread
From: Hari Bathini @ 2016-08-25 17:20 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: ast, lkml, acme, alexander.shishkin, mingo, sargun,
	Aravinda Prasad, Naveen N. Rao, Ananth N Mavinakayanahalli,
	daniel



On Thursday 25 August 2016 06:31 PM, Peter Zijlstra wrote:
> On Thu, Aug 25, 2016 at 05:27:54PM +0530, Hari Bathini wrote:
>
>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>> index c66a485..fb4f902 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -139,8 +139,9 @@ enum perf_event_sample_format {
>>   	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
>>   	PERF_SAMPLE_TRANSACTION			= 1U << 17,
>>   	PERF_SAMPLE_REGS_INTR			= 1U << 18,
>> +	PERF_SAMPLE_CID				= 1U << 19,
>>   
>> -	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
>> +	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
>>   };
> This forgets to update the comment that goes with PERF_RECORD_SAMPLE.
> This patch would also need an update to the manpage:
>
>    http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2
>
>    http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html
>
>> +	if (sample_type & PERF_SAMPLE_CID) {
>> +		int size = sizeof(u64);
>> +
>> +		/*
>> +		 * Container identifier for a given task.
>> +		 * Using cgroup namespace inode number for this.
>> +		 */
>> +		data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
>> +		data->cid_entry.reserved = 0;
>> +		header->size += size;
>> +	}
>>   }
> Does this compile with CONFIG_CGROUP=n ?
>

My bad. Will update..
Actually, on second thought, how about using inode number of some
other namespace that any container would have (mount, probably?)..

Thanks
Hari

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] perf tool: add container identifier entry related changes
  2016-08-25 13:04   ` Peter Zijlstra
@ 2016-08-25 17:22     ` Hari Bathini
  0 siblings, 0 replies; 8+ messages in thread
From: Hari Bathini @ 2016-08-25 17:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: ast, lkml, acme, alexander.shishkin, mingo, sargun,
	Aravinda Prasad, Naveen N. Rao, Ananth N Mavinakayanahalli,
	daniel



On Thursday 25 August 2016 06:34 PM, Peter Zijlstra wrote:
> On Thu, Aug 25, 2016 at 05:28:03PM +0530, Hari Bathini wrote:
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index d9b80ef..d35fbd3 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -929,6 +929,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
>>   	if (opts->sample_transaction)
>>   		perf_evsel__set_sample_bit(evsel, TRANSACTION);
>>   
>> +	perf_evsel__set_sample_bit(evsel, CID);
>> +
>>   	if (opts->running_time) {
>>   		evsel->attr.read_format |=
>>   			PERF_FORMAT_TOTAL_TIME_ENABLED |
> Does this mean its enabled by default? Why? By growing the default
> sample to include everything under the sun we make profiling slower for
> everyone.

True. Will put it under a flag..

- Hari

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] perf: add container identifier entry in perf sample data
  2016-08-25 17:20   ` Hari Bathini
@ 2016-08-25 17:31     ` Peter Zijlstra
  2016-08-26  4:05     ` Ananth N Mavinakayanahalli
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2016-08-25 17:31 UTC (permalink / raw)
  To: Hari Bathini
  Cc: ast, lkml, acme, alexander.shishkin, mingo, sargun,
	Aravinda Prasad, Naveen N. Rao, Ananth N Mavinakayanahalli,
	daniel

On Thu, Aug 25, 2016 at 10:50:18PM +0530, Hari Bathini wrote:

> Actually, on second thought, how about using inode number of some
> other namespace that any container would have (mount, probably?)..

I have no clue about those things. The only thing I can tell you is that
you get to pick once ;-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] perf: add container identifier entry in perf sample data
  2016-08-25 17:20   ` Hari Bathini
  2016-08-25 17:31     ` Peter Zijlstra
@ 2016-08-26  4:05     ` Ananth N Mavinakayanahalli
  1 sibling, 0 replies; 8+ messages in thread
From: Ananth N Mavinakayanahalli @ 2016-08-26  4:05 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Peter Zijlstra, ast, lkml, acme, alexander.shishkin, mingo,
	sargun, Aravinda Prasad, Naveen N. Rao, daniel

On Thu, Aug 25, 2016 at 10:50:18PM +0530, Hari Bathini wrote:
> 
> 
> On Thursday 25 August 2016 06:31 PM, Peter Zijlstra wrote:
> >On Thu, Aug 25, 2016 at 05:27:54PM +0530, Hari Bathini wrote:
> >
> >>diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> >>index c66a485..fb4f902 100644
> >>--- a/include/uapi/linux/perf_event.h
> >>+++ b/include/uapi/linux/perf_event.h
> >>@@ -139,8 +139,9 @@ enum perf_event_sample_format {
> >>  	PERF_SAMPLE_IDENTIFIER			= 1U << 16,
> >>  	PERF_SAMPLE_TRANSACTION			= 1U << 17,
> >>  	PERF_SAMPLE_REGS_INTR			= 1U << 18,
> >>+	PERF_SAMPLE_CID				= 1U << 19,
> >>-	PERF_SAMPLE_MAX = 1U << 19,		/* non-ABI */
> >>+	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
> >>  };
> >This forgets to update the comment that goes with PERF_RECORD_SAMPLE.
> >This patch would also need an update to the manpage:
> >
> >   http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2
> >
> >   http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html
> >
> >>+	if (sample_type & PERF_SAMPLE_CID) {
> >>+		int size = sizeof(u64);
> >>+
> >>+		/*
> >>+		 * Container identifier for a given task.
> >>+		 * Using cgroup namespace inode number for this.
> >>+		 */
> >>+		data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
> >>+		data->cid_entry.reserved = 0;
> >>+		header->size += size;
> >>+	}
> >>  }
> >Does this compile with CONFIG_CGROUP=n ?
> >
> 
> My bad. Will update..
> Actually, on second thought, how about using inode number of some
> other namespace that any container would have (mount, probably?)..

I am not sure about every implementation of 'containers' but I would
think that the cgroup namespace is the right choice here. Mount
namespaces can be potentially shared between containers AFAIK.

Ananth

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-08-26  4:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-25 11:57 [PATCH 1/2] perf: add container identifier entry in perf sample data Hari Bathini
2016-08-25 11:58 ` [PATCH 2/2] perf tool: add container identifier entry related changes Hari Bathini
2016-08-25 13:04   ` Peter Zijlstra
2016-08-25 17:22     ` Hari Bathini
2016-08-25 13:01 ` [PATCH 1/2] perf: add container identifier entry in perf sample data Peter Zijlstra
2016-08-25 17:20   ` Hari Bathini
2016-08-25 17:31     ` Peter Zijlstra
2016-08-26  4:05     ` Ananth N Mavinakayanahalli

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.