Messages in this thread | | | Subject | Re: [PATCH 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info | From | Hari Bathini <> | Date | Mon, 14 Nov 2016 16:02:30 +0530 |
| |
On Thursday 10 November 2016 06:49 PM, Peter Zijlstra wrote: > On Thu, Nov 10, 2016 at 05:08:06PM +0530, Hari Bathini wrote: > >> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h >> index c66a485..575aed6 100644 >> --- a/include/uapi/linux/perf_event.h >> +++ b/include/uapi/linux/perf_event.h >> @@ -344,7 +344,8 @@ struct perf_event_attr { >> use_clockid : 1, /* use @clockid for time fields */ >> context_switch : 1, /* context switch data */ >> write_backward : 1, /* Write ring buffer from end to beginning */ >> - __reserved_1 : 36; >> + namespaces : 1, /* include namespaces data */ >> + __reserved_1 : 35; >> >> union { >> __u32 wakeup_events; /* wakeup every n events */ >> @@ -862,6 +863,24 @@ enum perf_event_type { >> */ >> PERF_RECORD_SWITCH_CPU_WIDE = 15, >> >> + /* >> + * struct { >> + * struct perf_event_header header; >> + * >> + * u32 pid, tid; >> + * u64 time; >> + * u32 uts_ns_inum; >> + * u32 ipc_ns_inum; >> + * u32 mnt_ns_inum; >> + * u32 pid_ns_inum; >> + * u32 net_ns_inum; >> + * u32 cgroup_ns_inum; >> + * u32 user_ns_inum; >> + * struct sample_id sample_id; >> + * }; >> + */ >> + PERF_RECORD_NAMESPACES = 16, > So this format is not extensible, that is, if someone adds yet another > namespace, we'll need to introduce PERF_RECORD_NAMESPACES2. > > Is there a 'natural' and exposed namespace index that we can use to > change it like: > > u32 nr_nss; > u32 namespace[nr_nss]; > > ?
Hi Peter,
Nothing of that sort exists, currently. Maybe, time to introduce with this patch-set..?
>> +static void perf_event_namespaces_output(struct perf_event *event, >> + void *data) >> +{ >> + struct perf_namespaces_event *namespaces_event = data; >> + struct perf_output_handle handle; >> + struct perf_sample_data sample; >> + int size = namespaces_event->event_id.header.size; >> + struct nsproxy *nsproxy; >> + int ret; >> + >> + if (!perf_event_namespaces_match(event)) >> + return; >> + >> + perf_event_header__init_id(&namespaces_event->event_id.header, >> + &sample, event); >> + ret = perf_output_begin(&handle, event, >> + namespaces_event->event_id.header.size); >> + >> + if (ret) >> + goto out; > If you were to introduce: > > struct ns_event_id *ei = &namespace_event->event_id; > >> + >> + namespaces_event->event_id.pid = perf_event_pid(event, >> + namespaces_event->task); >> + namespaces_event->event_id.tid = perf_event_tid(event, >> + namespaces_event->task); >> + >> + if (namespaces_event->task != current) >> + task_lock(namespaces_event->task); >> + >> + nsproxy = namespaces_event->task->nsproxy; >> + if (nsproxy != NULL) { >> + namespaces_event->event_id.uts_ns_inum = >> + nsproxy->uts_ns->ns.inum; >> +#ifdef CONFIG_IPC_NS >> + namespaces_event->event_id.ipc_ns_inum = >> + nsproxy->ipc_ns->ns.inum; >> +#endif >> + namespaces_event->event_id.mnt_ns_inum = >> + nsproxy->mnt_ns->ns.inum; >> + namespaces_event->event_id.pid_ns_inum = >> + nsproxy->pid_ns_for_children->ns.inum; >> +#ifdef CONFIG_NET >> + namespaces_event->event_id.net_ns_inum = >> + nsproxy->net_ns->ns.inum; >> +#endif >> +#ifdef CONFIG_CGROUPS >> + namespaces_event->event_id.cgroup_ns_inum = >> + nsproxy->cgroup_ns->ns.inum; >> +#endif >> + } >> + >> + namespaces_event->event_id.user_ns_inum = >> + __task_cred(namespaces_event->task)->user_ns->ns.inum; > You can do s/namespace_event->event_id./ei->/ which is tons shorter and > would result in less wrapping of lines and generally improve > readability.
True.
>> + >> + if (namespaces_event->task != current) >> + task_unlock(namespaces_event->task); >> + >> + namespaces_event->event_id.time = perf_event_clock(event); >> + >> + perf_output_put(&handle, namespaces_event->event_id); >> + >> + perf_event__output_id_sample(event, &handle, &sample); >> + >> + perf_output_end(&handle); >> +out: >> + namespaces_event->event_id.header.size = size; >> +} >> + >> +void perf_event_namespaces(struct task_struct *task) >> +{ >> + struct perf_namespaces_event namespaces_event; >> + >> + if (!atomic_read(&nr_namespaces_events)) >> + return; >> + >> + namespaces_event = (struct perf_namespaces_event){ >> + .task = task, >> + .event_id = { >> + .header = { >> + .type = PERF_RECORD_NAMESPACES, >> + .misc = 0, >> + .size = sizeof(namespaces_event.event_id), >> + }, >> + /* .pid */ >> + /* .tid */ >> + /* .time */ >> + /* .uts_ns_inum */ >> + /* .ipc_ns_inum */ >> + /* .mnt_ns_inum */ >> + /* .pid_ns_inum */ >> + /* .net_ns_inum */ >> + /* .cgroup_ns_inum */ >> + /* .user_ns_inum */ >> + }, >> + }; >> + >> + perf_iterate_sb(perf_event_namespaces_output, >> + &namespaces_event, >> + NULL); >> +} >> diff --git a/kernel/fork.c b/kernel/fork.c >> index 997ac1d..3faca3d 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -1818,6 +1818,7 @@ static __latent_entropy struct task_struct *copy_process( >> cgroup_post_fork(p); >> threadgroup_change_end(current); >> perf_event_fork(p); >> + perf_event_namespaces(p); > I would much prefer calling perf_event_namespace() from > perf_event_fork() and reduce the external interface. >
OK. Will update..
Thanks Hari
| |