Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1470159 > unrolled thread

Re: [PATCH 1/2] perf: add container identifier entry in perf sample data

Started byPeter Zijlstra <peterz@infradead.org>
First post2016-08-25 15:50 +0200
Last post2016-08-26 06:10 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] perf: add container identifier entry in perf sample  data Peter Zijlstra <peterz@infradead.org> - 2016-08-25 15:50 +0200
    Re: [PATCH 1/2] perf: add container identifier entry in perf sample  data Peter Zijlstra <peterz@infradead.org> - 2016-08-25 19:40 +0200
    Re: [PATCH 1/2] perf: add container identifier entry in perf sample  data Hari Bathini <hbathini@linux.vnet.ibm.com> - 2016-08-26 01:00 +0200
      Re: [PATCH 1/2] perf: add container identifier entry in perf sample  data Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> - 2016-08-26 06:10 +0200

#1470159 — Re: [PATCH 1/2] perf: add container identifier entry in perf sample data

FromPeter Zijlstra <peterz@infradead.org>
Date2016-08-25 15:50 +0200
SubjectRe: [PATCH 1/2] perf: add container identifier entry in perf sample data
Message-ID<sa3cB-1AH-19@gated-at.bofh.it>
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 ?

[toc] | [next] | [standalone]

#1470299

FromPeter Zijlstra <peterz@infradead.org>
Date2016-08-25 19:40 +0200
Message-ID<sa6Nb-41J-5@gated-at.bofh.it>
In reply to#1470159
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 ;-)

[toc] | [prev] | [next] | [standalone]

#1470479

FromHari Bathini <hbathini@linux.vnet.ibm.com>
Date2016-08-26 01:00 +0200
Message-ID<sa6Nb-41J-7@gated-at.bofh.it>
In reply to#1470159

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

[toc] | [prev] | [next] | [standalone]

#1470536

FromAnanth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Date2016-08-26 06:10 +0200
Message-ID<sagCR-23I-3@gated-at.bofh.it>
In reply to#1470479
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web