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

Groups > linux.kernel > #1441176 > unrolled thread

Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace

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: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Aravinda Prasad <aravinda@linux.vnet.ibm.com> - 2016-07-12 11:30 +0200
    Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Peter Zijlstra <peterz@infradead.org> - 2016-07-12 13:50 +0200
      Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Nikolay Borisov <kernel@kyup.com> - 2016-07-12 14:50 +0200
        Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Peter Zijlstra <peterz@infradead.org> - 2016-07-12 15:10 +0200
          Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace ebiederm@xmission.com (Eric W. Biederman) - 2016-07-12 16:10 +0200
            Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Peter Zijlstra <peterz@infradead.org> - 2016-07-12 16:30 +0200
              Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Aravinda Prasad <aravinda@linux.vnet.ibm.com> - 2016-07-12 18:10 +0200
          Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace Aravinda Prasad <aravinda@linux.vnet.ibm.com> - 2016-07-12 18:30 +0200

#1441176 — Re: [RFC PATCH 2/2] perf: Filter events based on perf-namespace

On Monday 27 June 2016 09:20 PM, Peter Zijlstra wrote:
> On Tue, Jun 14, 2016 at 10:19:51PM +0530, Aravinda Prasad wrote:
>> Whenever perf tool is executed inside a container, this
>> patch restricts the events to the perf-namespace in which
>> the perf tool is executing.
>>
>> This patch is based on the existing support available
>> for tracing with cgroups.
>>
>> TODO:
>>     - Avoid code duplication.
> 
> Can't you, at perf_event_open() time, convert a per-cpu event into a
> per-cpu-per-cgroup event for these namespace thingies?
> 
> That seems to immediately and completely remove all that duplication.

Sorry for the delay in the response.

I was looking into a way how a per-cpu event can be converted to
per-cpu-per-cgroup event at perf_event_open() but could not figure out
how to do this conversion. The cgroup event expects the fd of the cgroup
directory in cgroupfs and in this case we don't have any fd passed in.
Not sure if I am missing anything.

-- 
Regards,
Aravinda

[toc] | [next] | [standalone]

#1441278

On Tue, Jul 12, 2016 at 02:56:17PM +0530, Aravinda Prasad wrote:
> 
> 
> On Monday 27 June 2016 09:20 PM, Peter Zijlstra wrote:
> > On Tue, Jun 14, 2016 at 10:19:51PM +0530, Aravinda Prasad wrote:
> >> Whenever perf tool is executed inside a container, this
> >> patch restricts the events to the perf-namespace in which
> >> the perf tool is executing.
> >>
> >> This patch is based on the existing support available
> >> for tracing with cgroups.
> >>
> >> TODO:
> >>     - Avoid code duplication.
> > 
> > Can't you, at perf_event_open() time, convert a per-cpu event into a
> > per-cpu-per-cgroup event for these namespace thingies?
> > 
> > That seems to immediately and completely remove all that duplication.
> 
> Sorry for the delay in the response.
> 
> I was looking into a way how a per-cpu event can be converted to
> per-cpu-per-cgroup event at perf_event_open() but could not figure out
> how to do this conversion. The cgroup event expects the fd of the cgroup
> directory in cgroupfs and in this case we don't have any fd passed in.
> Not sure if I am missing anything.

Would not the current namespace have a link to the correct cgroup?
Wasn't that the entire point of the namespace thing?

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

#1441321

On 07/12/2016 02:47 PM, Peter Zijlstra wrote:
> On Tue, Jul 12, 2016 at 02:56:17PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Monday 27 June 2016 09:20 PM, Peter Zijlstra wrote:
>>> On Tue, Jun 14, 2016 at 10:19:51PM +0530, Aravinda Prasad wrote:
>>>> Whenever perf tool is executed inside a container, this
>>>> patch restricts the events to the perf-namespace in which
>>>> the perf tool is executing.
>>>>
>>>> This patch is based on the existing support available
>>>> for tracing with cgroups.
>>>>
>>>> TODO:
>>>>     - Avoid code duplication.
>>>
>>> Can't you, at perf_event_open() time, convert a per-cpu event into a
>>> per-cpu-per-cgroup event for these namespace thingies?
>>>
>>> That seems to immediately and completely remove all that duplication.
>>
>> Sorry for the delay in the response.
>>
>> I was looking into a way how a per-cpu event can be converted to
>> per-cpu-per-cgroup event at perf_event_open() but could not figure out
>> how to do this conversion. The cgroup event expects the fd of the cgroup
>> directory in cgroupfs and in this case we don't have any fd passed in.
>> Not sure if I am missing anything.
> 
> Would not the current namespace have a link to the correct cgroup?
> Wasn't that the entire point of the namespace thing?

Namespaces and cgroups are completely orthogonal to one another. Also in
the v1 of cgroups it's possible to have a process member of more than 1
cgroup.

> 

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

#1441338

On Tue, Jul 12, 2016 at 03:41:42PM +0300, Nikolay Borisov wrote:

> Namespaces and cgroups are completely orthogonal to one another. 

Then how do you specify what your new 'root' is? Surely you must first
create a cgroup and then confine yourself to that?

> Also in the v1 of cgroups it's possible to have a process member of
> more than 1 cgroup.

Yeah, so? We only care about the perf controller obviously.

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

#1441381

Peter Zijlstra <peterz@infradead.org> writes:

> On Tue, Jul 12, 2016 at 03:41:42PM +0300, Nikolay Borisov wrote:
>
>> Namespaces and cgroups are completely orthogonal to one another. 
>
> Then how do you specify what your new 'root' is? Surely you must first
> create a cgroup and then confine yourself to that?
>
>> Also in the v1 of cgroups it's possible to have a process member of
>> more than 1 cgroup.
>
> Yeah, so? We only care about the perf controller obviously.

I completely misread the description of this, or I would have something
earlier.  For some reason I thought he was talking about the perf
controller.

As I recall the tricky part of this was to have tracing that was safe
and usable inside of a container.  If you can align a per cgroup with
your container that is probably sufficient for the select of processes.

At the same time there is a real desire to have identifiers like pids
translated into the appropriate form for inside of the container.
Without that translation they are meaningless inside a container.
Further it is necessary to be certain the trancing that is used is is
safe for unprivileged users.

I don't think I ever suggested or approved of the concept of a perf
namespace and that sounds a bit dubious to me.

Eric

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

#1441415

On Tue, Jul 12, 2016 at 08:55:17AM -0500, Eric W. Biederman wrote:

> I completely misread the description of this, or I would have something
> earlier.  For some reason I thought he was talking about the perf
> controller.
> 
> As I recall the tricky part of this was to have tracing that was safe
> and usable inside of a container.  If you can align a per cgroup with
> your container that is probably sufficient for the select of processes.
> 
> At the same time there is a real desire to have identifiers like pids
> translated into the appropriate form for inside of the container.
> Without that translation they are meaningless inside a container.
> Further it is necessary to be certain the trancing that is used is is
> safe for unprivileged users.
> 
> I don't think I ever suggested or approved of the concept of a perf
> namespace and that sounds a bit dubious to me.

So perf uses the pid-namespace of the event-creator to report PID/TID
numbers in.

So sys_perf_event_open() -> perf_event_alloc() does
get_pid_ns(task_active_pid_ns(current)) to set event->ns and then we do:
task_{tgid,pid}_nr_ns(p, event->ns) to report the PID/TID resp., see
perf_event_{pid,tid}().

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

#1441528

On Tuesday 12 July 2016 07:57 PM, Peter Zijlstra wrote:
> On Tue, Jul 12, 2016 at 08:55:17AM -0500, Eric W. Biederman wrote:
> 
>> I completely misread the description of this, or I would have something
>> earlier.  For some reason I thought he was talking about the perf
>> controller.
>>
>> As I recall the tricky part of this was to have tracing that was safe
>> and usable inside of a container.  If you can align a per cgroup with
>> your container that is probably sufficient for the select of processes.
>>

Aligning a cgroup with the container is sufficient if containers are
created with PID namespace. The first prototype was based on that.

However, not sure if it is fair to assume that the containers are
created with PID namespace and the processes inside the container are
grouped into a cgroup, as containers can be created without PID
namespace. In fact it was mentioned in LPC container micro-conference
that some containers are created without PID namespace as they need to
access host PIDs.

With the recent introduction of cgroup namespace, I think we can even
take out the requirement that container should be created with PID
namespace to enable safe tracing inside of a container. We are currently
evaluating that.


>> At the same time there is a real desire to have identifiers like pids
>> translated into the appropriate form for inside of the container.
>> Without that translation they are meaningless inside a container.
>> Further it is necessary to be certain the trancing that is used is is
>> safe for unprivileged users.

pid is already translated inside a container as mentioned below.

>>
>> I don't think I ever suggested or approved of the concept of a perf
>> namespace and that sounds a bit dubious to me.

Yes true this was not suggested during the discussion.

As the kernel does not have the concept of a container, we thought
introduction of perf-namespace could isolate events inside a container,
analogous to other namespaces.

> 
> So perf uses the pid-namespace of the event-creator to report PID/TID
> numbers in.
> 
> So sys_perf_event_open() -> perf_event_alloc() does
> get_pid_ns(task_active_pid_ns(current)) to set event->ns and then we do:
> task_{tgid,pid}_nr_ns(p, event->ns) to report the PID/TID resp., see
> perf_event_{pid,tid}().
> 

-- 
Regards,
Aravinda

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

#1441536

On Tuesday 12 July 2016 06:34 PM, Peter Zijlstra wrote:
> On Tue, Jul 12, 2016 at 03:41:42PM +0300, Nikolay Borisov wrote:
> 
>> Namespaces and cgroups are completely orthogonal to one another. 
> 
> Then how do you specify what your new 'root' is? Surely you must first
> create a cgroup and then confine yourself to that?

No need to create cgroup and confine processes to that cgroup. The below
check during event filtering will take care:

+	if (cpuctx->perf_ns != event->perf_ns)
+		return false;

event->perf_ns is set during perf_event_alloc():

+	if (current->nsproxy->perf_ns != &init_perf_ns) {
+		/*
+		 * If we are called from our own perf namespace, set
+		 * event->perf_ns
+		 */
+		event->perf_ns = current->nsproxy->perf_ns;
+		[...]
+	}

> 
>> Also in the v1 of cgroups it's possible to have a process member of
>> more than 1 cgroup.
> 
> Yeah, so? We only care about the perf controller obviously.
> 

-- 
Regards,
Aravinda

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web