Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-12-12 Thread Hari Bathini 

Hi Peter,


On Friday 09 December 2016 07:39 PM, Peter Zijlstra wrote:

On Fri, Dec 09, 2016 at 12:10:20AM +0530, Hari Bathini wrote:

Hi Peter,


Sorry for taking so long to respond...


On Thursday 24 November 2016 08:40 PM, Peter Zijlstra wrote:

On Thu, Nov 24, 2016 at 08:14:29PM +0530, Hari Bathini wrote:

@@ -862,6 +875,19 @@ enum perf_event_type {
 */
PERF_RECORD_SWITCH_CPU_WIDE = 15,
+   /*
+* struct {
+*  struct perf_event_headerheader;
+*
+*  u32 pid, tid;
+*  u64 time;

pid,tid and time are already present in sample_id. Many of the 'legacy'
record have redundant information since we added sample_id, but most of
the new ones haven't and rely on sample_all being set.

I tried using pid/tid from sample data, but realized that pid/tid in
event_id
could be different from the one in sample data, at least for fork/namespaces
events, since __perf_event_header__init_id( ) that updates the sample data
is getting the pid/tid of current task.

Ah indeed. Yes please disregard my comment then.


Posted v3 addressing the TODOs & fixing the error reported by kbuild 
test robot.


Thanks
Hari

Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-12-09 Thread Peter Zijlstra 
On Fri, Dec 09, 2016 at 12:10:20AM +0530, Hari Bathini wrote:
> Hi Peter,
> 
> 
> Sorry for taking so long to respond...
> 
> 
> On Thursday 24 November 2016 08:40 PM, Peter Zijlstra wrote:
> >On Thu, Nov 24, 2016 at 08:14:29PM +0530, Hari Bathini wrote:
> >>@@ -862,6 +875,19 @@ enum perf_event_type {
> >> */
> >>PERF_RECORD_SWITCH_CPU_WIDE = 15,
> >>+   /*
> >>+* struct {
> >>+*  struct perf_event_headerheader;
> >>+*
> >>+*  u32 pid, tid;
> >>+*  u64 time;
> >pid,tid and time are already present in sample_id. Many of the 'legacy'
> >record have redundant information since we added sample_id, but most of
> >the new ones haven't and rely on sample_all being set.
> 
> I tried using pid/tid from sample data, but realized that pid/tid in
> event_id
> could be different from the one in sample data, at least for fork/namespaces
> events, since __perf_event_header__init_id( ) that updates the sample data
> is getting the pid/tid of current task.

Ah indeed. Yes please disregard my comment then.

Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-12-09 Thread Hari Bathini 



On Friday 09 December 2016 12:10 AM, Hari Bathini wrote:

Hi Peter,


Sorry for taking so long to respond...


On Thursday 24 November 2016 08:40 PM, Peter Zijlstra wrote:

On Thu, Nov 24, 2016 at 08:14:29PM +0530, Hari Bathini wrote:

@@ -862,6 +875,19 @@ enum perf_event_type {
   */
  PERF_RECORD_SWITCH_CPU_WIDE= 15,
  +/*
+ * struct {
+ *struct perf_event_headerheader;
+ *
+ *u32pid, tid;
+ *u64time;

pid,tid and time are already present in sample_id. Many of the 'legacy'
record have redundant information since we added sample_id, but most of
the new ones haven't and rely on sample_all being set.


I tried using pid/tid from sample data, but realized that pid/tid in 
event_id
could be different from the one in sample data, at least for 
fork/namespaces
events, since __perf_event_header__init_id( ) that updates the sample 
data

is getting the pid/tid of current task.

I am not sure if it is advisable to change 
__perf_event_header__init_id( ) for this..?




Hi Peter,

Adding task parameter to __perf_event_header__init_id( ) and doing 
something like..


perf_event_pid(event, task ? task : current)
perf_event_tid(event, task ? task : current)


Also, for synthesized events, sample_id is not updated currently. 
Working on it as that
is needed if we want to relay on pid, tid values from sample_id. Let me 
know, if this is

what you had in mind or something else altogether..

Thanks
Hari

Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-12-08 Thread Hari Bathini 

Hi Peter,


Sorry for taking so long to respond...


On Thursday 24 November 2016 08:40 PM, Peter Zijlstra wrote:

On Thu, Nov 24, 2016 at 08:14:29PM +0530, Hari Bathini wrote:

@@ -862,6 +875,19 @@ enum perf_event_type {
 */
PERF_RECORD_SWITCH_CPU_WIDE = 15,
  
+	/*

+* struct {
+*  struct perf_event_headerheader;
+*
+*  u32 pid, tid;
+*  u64 time;

pid,tid and time are already present in sample_id. Many of the 'legacy'
record have redundant information since we added sample_id, but most of
the new ones haven't and rely on sample_all being set.


I tried using pid/tid from sample data, but realized that pid/tid in 
event_id

could be different from the one in sample data, at least for fork/namespaces
events, since __perf_event_header__init_id( ) that updates the sample data
is getting the pid/tid of current task.

I am not sure if it is advisable to change __perf_event_header__init_id( 
) for this..?


Thanks
Hari

Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-11-24 Thread kbuild test robot 
Hi Hari,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hari-Bathini/perf-add-support-for-analyzing-events-for-containers/20161125-061105
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   kernel/fork.c: In function 'SYSC_unshare':
>> kernel/fork.c:2284:3: error: implicit declaration of function 
>> 'perf_event_namespaces' [-Werror=implicit-function-declaration]
  perf_event_namespaces(current);
  ^
   cc1: some warnings being treated as errors
--
   kernel/nsproxy.c: In function 'SYSC_setns':
>> kernel/nsproxy.c:270:3: error: implicit declaration of function 
>> 'perf_event_namespaces' [-Werror=implicit-function-declaration]
  perf_event_namespaces(tsk);
  ^
   cc1: some warnings being treated as errors

vim +/perf_event_namespaces +2284 kernel/fork.c

  2278  bad_unshare_cleanup_fs:
  2279  if (new_fs)
  2280  free_fs_struct(new_fs);
  2281  
  2282  bad_unshare_out:
  2283  if (!err)
> 2284  perf_event_namespaces(current);
  2285  
  2286  return err;
  2287  }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 1/3] perf: add PERF_RECORD_NAMESPACES to include namespaces related info

2016-11-24 Thread Peter Zijlstra 
On Thu, Nov 24, 2016 at 08:14:29PM +0530, Hari Bathini wrote:
> @@ -862,6 +875,19 @@ enum perf_event_type {
>*/
>   PERF_RECORD_SWITCH_CPU_WIDE = 15,
>  
> + /*
> +  * struct {
> +  *  struct perf_event_headerheader;
> +  *

> +  *  u32 pid, tid;
> +  *  u64 time;

pid,tid and time are already present in sample_id. Many of the 'legacy'
record have redundant information since we added sample_id, but most of
the new ones haven't and rely on sample_all being set.

> +  *  u64 dev_num;
> +  *  u64 inode_num[NAMESPACES_MAX];
> +  *  struct sample_idsample_id;
> +  * };
> +  */
> + PERF_RECORD_NAMESPACES  = 16,
> +
>   PERF_RECORD_MAX,/* non-ABI */
>  };