RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-19 Thread Liang, Kan


> 
> On Tue, 18 Nov 2014 14:01:06 +, Kan Liang wrote:
> >> On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
> >> > +/* LBR only affects the user callchain */
> >> > +if (i != chain_nr) {
> >> > +struct branch_stack *lbr_stack = sample-
> >> >branch_stack;
> >> > +int lbr_nr = lbr_stack->nr;
> >> > +/*
> >> > + * LBR callstack can only get user call chain.
> >> > + * The mix_chain_nr is kernel call chain
> >> > + * number plus LBR user call chain number.
> >> > + * i is kernel call chain number,
> >> > + * 1 is PERF_CONTEXT_USER,
> >> > + * lbr_nr + 1 is the user call chain number.
> >> > + * For details, please refer to the comments
> >> > + * in callchain__printf
> >> > + */
> >> > +int mix_chain_nr = i + 1 + lbr_nr + 1;
> >> > +
> >> > +if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
> >> > +pr_warning("corrupted callchain.
> >> skipping...\n");
> >> > +return 0;
> >> > +}
> >>
> >> I'm not sure whether this is really a corrupted callchain.  If a
> >> single chain is greater than the max it should be corrupted, but
> >> we're now summing up with other values..
> >>
> >> What about checking callchain_nr and lbr_nr separately or mix_nr with
> >> 2*max?
> >
> > The lbr_nr max is 16. I will change it to max+16.
> 
> Shouldn't it be 18 (16 + 1 + 1) at least?

In the new patch, I use PERF_MAX_BRANCH_DEPTH from Andi's patch. It's
127. It should be enough.

 +  if (mix_chain_nr > PERF_MAX_STACK_DEPTH +
 PERF_MAX_BRANCH_DEPTH) {
 +  pr_warning("corrupted callchain. skipping...\n");
 +  return 0;
 +  }

Thanks,
Kan
> 
> Thanks,
> Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-19 Thread Liang, Kan


 
 On Tue, 18 Nov 2014 14:01:06 +, Kan Liang wrote:
  On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
   +/* LBR only affects the user callchain */
   +if (i != chain_nr) {
   +struct branch_stack *lbr_stack = sample-
  branch_stack;
   +int lbr_nr = lbr_stack-nr;
   +/*
   + * LBR callstack can only get user call chain.
   + * The mix_chain_nr is kernel call chain
   + * number plus LBR user call chain number.
   + * i is kernel call chain number,
   + * 1 is PERF_CONTEXT_USER,
   + * lbr_nr + 1 is the user call chain number.
   + * For details, please refer to the comments
   + * in callchain__printf
   + */
   +int mix_chain_nr = i + 1 + lbr_nr + 1;
   +
   +if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
   +pr_warning(corrupted callchain.
  skipping...\n);
   +return 0;
   +}
 
  I'm not sure whether this is really a corrupted callchain.  If a
  single chain is greater than the max it should be corrupted, but
  we're now summing up with other values..
 
  What about checking callchain_nr and lbr_nr separately or mix_nr with
  2*max?
 
  The lbr_nr max is 16. I will change it to max+16.
 
 Shouldn't it be 18 (16 + 1 + 1) at least?

In the new patch, I use PERF_MAX_BRANCH_DEPTH from Andi's patch. It's
127. It should be enough.

 +  if (mix_chain_nr  PERF_MAX_STACK_DEPTH +
 PERF_MAX_BRANCH_DEPTH) {
 +  pr_warning(corrupted callchain. skipping...\n);
 +  return 0;
 +  }

Thanks,
Kan
 
 Thanks,
 Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Namhyung Kim
On Tue, 18 Nov 2014 14:01:06 +, Kan Liang wrote:
>> On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
>> > +  /* LBR only affects the user callchain */
>> > +  if (i != chain_nr) {
>> > +  struct branch_stack *lbr_stack = sample-
>> >branch_stack;
>> > +  int lbr_nr = lbr_stack->nr;
>> > +  /*
>> > +   * LBR callstack can only get user call chain.
>> > +   * The mix_chain_nr is kernel call chain
>> > +   * number plus LBR user call chain number.
>> > +   * i is kernel call chain number,
>> > +   * 1 is PERF_CONTEXT_USER,
>> > +   * lbr_nr + 1 is the user call chain number.
>> > +   * For details, please refer to the comments
>> > +   * in callchain__printf
>> > +   */
>> > +  int mix_chain_nr = i + 1 + lbr_nr + 1;
>> > +
>> > +  if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
>> > +  pr_warning("corrupted callchain.
>> skipping...\n");
>> > +  return 0;
>> > +  }
>> 
>> I'm not sure whether this is really a corrupted callchain.  If a single 
>> chain is
>> greater than the max it should be corrupted, but we're now summing up
>> with other values..
>> 
>> What about checking callchain_nr and lbr_nr separately or mix_nr with
>> 2*max?
>
> The lbr_nr max is 16. I will change it to max+16.

Shouldn't it be 18 (16 + 1 + 1) at least?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Namhyung Kim
Hi Kan,

On Tue, 18 Nov 2014 19:40:23 +, Kan Liang wrote:
>> > whole
>> > > >> stack.
>> > > >> > +*/
>> > > >>
>> > > >> Andi is using some sanity checks:
>> > > >> http://marc.info/?l=linux-kernel=141584447819894=2
>> > > >> I guess this could be applied in here, once his patch gets in.
>> > > >>
>> > > >
>> > > > Are you suggesting me to remove the comments, or rebase the
>> whole
>> > > > patch to Andi's patch once it's merged?
>> > > >
>> > > > The branch history in Andi's patch is different as the call stack,
>> > > > although they are both from LBR.
>> > > > Andi's branch history recording branch records for taken branches,
>> > > > interrupts, and exceptions.
>> > > > While the LBR call stack records for the call stack.
>> > >
>> > > Right.  And branch history can overlap with normal callchains so
>> > > additional check in there is to remove duplication.  While LBR call
>> > > stack is separated to user only so there should be no overlap.
>> >
>> > hum, it seemed to me like the remove_loops function could be used for
>> > this one as well.. but anyway I meant that this can be introduced
>> > later after Andi's change gets in
>> 
>> I see. I will apply Andi's remove_loops.
>> 
>
> As Namhyung said, there is no overlap for LBR call stack. The user callchain 
> is not a mix. It's from either LBR or FP. so remove_loops
> doesn't fit to the LBR call stack. 

IIUC the remove_loops() is for eliminating repeated branches in a loop
in a single function (and it only checks LBRs not FP callchain).  It's
needed since branch history saves all branches, but not needed for LBR
callstack as it saves CALLs only.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


> > whole
> > > >> stack.
> > > >> > + */
> > > >>
> > > >> Andi is using some sanity checks:
> > > >> http://marc.info/?l=linux-kernel=141584447819894=2
> > > >> I guess this could be applied in here, once his patch gets in.
> > > >>
> > > >
> > > > Are you suggesting me to remove the comments, or rebase the
> whole
> > > > patch to Andi's patch once it's merged?
> > > >
> > > > The branch history in Andi's patch is different as the call stack,
> > > > although they are both from LBR.
> > > > Andi's branch history recording branch records for taken branches,
> > > > interrupts, and exceptions.
> > > > While the LBR call stack records for the call stack.
> > >
> > > Right.  And branch history can overlap with normal callchains so
> > > additional check in there is to remove duplication.  While LBR call
> > > stack is separated to user only so there should be no overlap.
> >
> > hum, it seemed to me like the remove_loops function could be used for
> > this one as well.. but anyway I meant that this can be introduced
> > later after Andi's change gets in
> 
> I see. I will apply Andi's remove_loops.
> 

As Namhyung said, there is no overlap for LBR call stack. The user callchain 
is not a mix. It's from either LBR or FP. so remove_loops
doesn't fit to the LBR call stack. 
Sorry for the confusion from my last reply.

Thanks,
Kan  
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


> On Tue, Nov 18, 2014 at 03:13:50PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > >> > +   * in "from" register, while the callee is
> stored
> > >> > +   * in "to" register.
> > >> > +   * For example, there is a call stack
> > >> > +   * "A"->"B"->"C"->"D".
> > >> > +   * The LBR registers will recorde like
> > >> > +   * "C"->"D", "B"->"C", "A"->"B".
> > >> > +   * So only the first "to" register and all
> "from"
> > >> > +   * registers are needed to construct the
> whole
> > >> stack.
> > >> > +   */
> > >>
> > >> Andi is using some sanity checks:
> > >> http://marc.info/?l=linux-kernel=141584447819894=2
> > >> I guess this could be applied in here, once his patch gets in.
> > >>
> > >
> > > Are you suggesting me to remove the comments, or rebase the whole
> > > patch to Andi's patch once it's merged?
> > >
> > > The branch history in Andi's patch is different as the call stack,
> > > although they are both from LBR.
> > > Andi's branch history recording branch records for taken branches,
> > > interrupts, and exceptions.
> > > While the LBR call stack records for the call stack.
> >
> > Right.  And branch history can overlap with normal callchains so
> > additional check in there is to remove duplication.  While LBR call
> > stack is separated to user only so there should be no overlap.
> 
> hum, it seemed to me like the remove_loops function could be used for
> this one as well.. but anyway I meant that this can be introduced later after
> Andi's change gets in

I see. I will apply Andi's remove_loops.

Thanks,
Kan

> 
> thanks,
> jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


> On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
> > +   /* LBR only affects the user callchain */
> > +   if (i != chain_nr) {
> > +   struct branch_stack *lbr_stack = sample-
> >branch_stack;
> > +   int lbr_nr = lbr_stack->nr;
> > +   /*
> > +* LBR callstack can only get user call chain.
> > +* The mix_chain_nr is kernel call chain
> > +* number plus LBR user call chain number.
> > +* i is kernel call chain number,
> > +* 1 is PERF_CONTEXT_USER,
> > +* lbr_nr + 1 is the user call chain number.
> > +* For details, please refer to the comments
> > +* in callchain__printf
> > +*/
> > +   int mix_chain_nr = i + 1 + lbr_nr + 1;
> > +
> > +   if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
> > +   pr_warning("corrupted callchain.
> skipping...\n");
> > +   return 0;
> > +   }
> 
> I'm not sure whether this is really a corrupted callchain.  If a single chain 
> is
> greater than the max it should be corrupted, but we're now summing up
> with other values..
> 
> What about checking callchain_nr and lbr_nr separately or mix_nr with
> 2*max?

The lbr_nr max is 16. I will change it to max+16.

Thanks,
Kan

> 
> Thanks,
> Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


 On Tue, Nov 18, 2014 at 03:13:50PM +0900, Namhyung Kim wrote:
 
 SNIP
 
+   * in from register, while the callee is
 stored
+   * in to register.
+   * For example, there is a call stack
+   * A-B-C-D.
+   * The LBR registers will recorde like
+   * C-D, B-C, A-B.
+   * So only the first to register and all
 from
+   * registers are needed to construct the
 whole
   stack.
+   */
  
   Andi is using some sanity checks:
   http://marc.info/?l=linux-kernelm=141584447819894w=2
   I guess this could be applied in here, once his patch gets in.
  
  
   Are you suggesting me to remove the comments, or rebase the whole
   patch to Andi's patch once it's merged?
  
   The branch history in Andi's patch is different as the call stack,
   although they are both from LBR.
   Andi's branch history recording branch records for taken branches,
   interrupts, and exceptions.
   While the LBR call stack records for the call stack.
 
  Right.  And branch history can overlap with normal callchains so
  additional check in there is to remove duplication.  While LBR call
  stack is separated to user only so there should be no overlap.
 
 hum, it seemed to me like the remove_loops function could be used for
 this one as well.. but anyway I meant that this can be introduced later after
 Andi's change gets in

I see. I will apply Andi's remove_loops.

Thanks,
Kan

 
 thanks,
 jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


  whole
stack.
 + */
   
Andi is using some sanity checks:
http://marc.info/?l=linux-kernelm=141584447819894w=2
I guess this could be applied in here, once his patch gets in.
   
   
Are you suggesting me to remove the comments, or rebase the
 whole
patch to Andi's patch once it's merged?
   
The branch history in Andi's patch is different as the call stack,
although they are both from LBR.
Andi's branch history recording branch records for taken branches,
interrupts, and exceptions.
While the LBR call stack records for the call stack.
  
   Right.  And branch history can overlap with normal callchains so
   additional check in there is to remove duplication.  While LBR call
   stack is separated to user only so there should be no overlap.
 
  hum, it seemed to me like the remove_loops function could be used for
  this one as well.. but anyway I meant that this can be introduced
  later after Andi's change gets in
 
 I see. I will apply Andi's remove_loops.
 

As Namhyung said, there is no overlap for LBR call stack. The user callchain 
is not a mix. It's from either LBR or FP. so remove_loops
doesn't fit to the LBR call stack. 
Sorry for the confusion from my last reply.

Thanks,
Kan  
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Namhyung Kim
Hi Kan,

On Tue, 18 Nov 2014 19:40:23 +, Kan Liang wrote:
  whole
stack.
 +*/
   
Andi is using some sanity checks:
http://marc.info/?l=linux-kernelm=141584447819894w=2
I guess this could be applied in here, once his patch gets in.
   
   
Are you suggesting me to remove the comments, or rebase the
 whole
patch to Andi's patch once it's merged?
   
The branch history in Andi's patch is different as the call stack,
although they are both from LBR.
Andi's branch history recording branch records for taken branches,
interrupts, and exceptions.
While the LBR call stack records for the call stack.
  
   Right.  And branch history can overlap with normal callchains so
   additional check in there is to remove duplication.  While LBR call
   stack is separated to user only so there should be no overlap.
 
  hum, it seemed to me like the remove_loops function could be used for
  this one as well.. but anyway I meant that this can be introduced
  later after Andi's change gets in
 
 I see. I will apply Andi's remove_loops.
 

 As Namhyung said, there is no overlap for LBR call stack. The user callchain 
 is not a mix. It's from either LBR or FP. so remove_loops
 doesn't fit to the LBR call stack. 

IIUC the remove_loops() is for eliminating repeated branches in a loop
in a single function (and it only checks LBRs not FP callchain).  It's
needed since branch history saves all branches, but not needed for LBR
callstack as it saves CALLs only.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Namhyung Kim
On Tue, 18 Nov 2014 14:01:06 +, Kan Liang wrote:
 On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
  +  /* LBR only affects the user callchain */
  +  if (i != chain_nr) {
  +  struct branch_stack *lbr_stack = sample-
 branch_stack;
  +  int lbr_nr = lbr_stack-nr;
  +  /*
  +   * LBR callstack can only get user call chain.
  +   * The mix_chain_nr is kernel call chain
  +   * number plus LBR user call chain number.
  +   * i is kernel call chain number,
  +   * 1 is PERF_CONTEXT_USER,
  +   * lbr_nr + 1 is the user call chain number.
  +   * For details, please refer to the comments
  +   * in callchain__printf
  +   */
  +  int mix_chain_nr = i + 1 + lbr_nr + 1;
  +
  +  if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
  +  pr_warning(corrupted callchain.
 skipping...\n);
  +  return 0;
  +  }
 
 I'm not sure whether this is really a corrupted callchain.  If a single 
 chain is
 greater than the max it should be corrupted, but we're now summing up
 with other values..
 
 What about checking callchain_nr and lbr_nr separately or mix_nr with
 2*max?

 The lbr_nr max is 16. I will change it to max+16.

Shouldn't it be 18 (16 + 1 + 1) at least?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-18 Thread Liang, Kan


 On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
  +   /* LBR only affects the user callchain */
  +   if (i != chain_nr) {
  +   struct branch_stack *lbr_stack = sample-
 branch_stack;
  +   int lbr_nr = lbr_stack-nr;
  +   /*
  +* LBR callstack can only get user call chain.
  +* The mix_chain_nr is kernel call chain
  +* number plus LBR user call chain number.
  +* i is kernel call chain number,
  +* 1 is PERF_CONTEXT_USER,
  +* lbr_nr + 1 is the user call chain number.
  +* For details, please refer to the comments
  +* in callchain__printf
  +*/
  +   int mix_chain_nr = i + 1 + lbr_nr + 1;
  +
  +   if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
  +   pr_warning(corrupted callchain.
 skipping...\n);
  +   return 0;
  +   }
 
 I'm not sure whether this is really a corrupted callchain.  If a single chain 
 is
 greater than the max it should be corrupted, but we're now summing up
 with other values..
 
 What about checking callchain_nr and lbr_nr separately or mix_nr with
 2*max?

The lbr_nr max is 16. I will change it to max+16.

Thanks,
Kan

 
 Thanks,
 Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Tue, Nov 18, 2014 at 03:13:50PM +0900, Namhyung Kim wrote:

SNIP

> >> > + * in "from" register, while the callee is 
> >> > stored
> >> > + * in "to" register.
> >> > + * For example, there is a call stack
> >> > + * "A"->"B"->"C"->"D".
> >> > + * The LBR registers will recorde like
> >> > + * "C"->"D", "B"->"C", "A"->"B".
> >> > + * So only the first "to" register and all 
> >> > "from"
> >> > + * registers are needed to construct the whole
> >> stack.
> >> > + */
> >> 
> >> Andi is using some sanity checks:
> >> http://marc.info/?l=linux-kernel=141584447819894=2
> >> I guess this could be applied in here, once his patch gets in.
> >> 
> >
> > Are you suggesting me to remove the comments,
> > or rebase the whole patch to Andi's patch once it's merged?
> >
> > The branch history in Andi's patch is different as the call stack,
> > although they are both from LBR.
> > Andi's branch history recording branch records for
> > taken branches, interrupts, and exceptions.
> > While the LBR call stack records for the call stack.
> 
> Right.  And branch history can overlap with normal callchains so
> additional check in there is to remove duplication.  While LBR call
> stack is separated to user only so there should be no overlap.

hum, it seemed to me like the remove_loops function could be used
for this one as well.. but anyway I meant that this can be introduced
later after Andi's change gets in

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
> + /* LBR only affects the user callchain */
> + if (i != chain_nr) {
> + struct branch_stack *lbr_stack = sample->branch_stack;
> + int lbr_nr = lbr_stack->nr;
> + /*
> +  * LBR callstack can only get user call chain.
> +  * The mix_chain_nr is kernel call chain
> +  * number plus LBR user call chain number.
> +  * i is kernel call chain number,
> +  * 1 is PERF_CONTEXT_USER,
> +  * lbr_nr + 1 is the user call chain number.
> +  * For details, please refer to the comments
> +  * in callchain__printf
> +  */
> + int mix_chain_nr = i + 1 + lbr_nr + 1;
> +
> + if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
> + pr_warning("corrupted callchain. 
> skipping...\n");
> + return 0;
> + }

I'm not sure whether this is really a corrupted callchain.  If a single
chain is greater than the max it should be corrupted, but we're now
summing up with other values..

What about checking callchain_nr and lbr_nr separately or mix_nr with
2*max?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Mon, 17 Nov 2014 17:41:32 +, Kan Liang wrote:
>> SNIP
>> 
>> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> > index f4478ce..335c3a9 100644
>> > --- a/tools/perf/util/session.c
>> > +++ b/tools/perf/util/session.c
>> > @@ -557,15 +557,63 @@ int perf_session_queue_event(struct
>> perf_session *s, union perf_event *event,
>> >return 0;
>> >  }
>> >
>> > -static void callchain__printf(struct perf_sample *sample)
>> > +static void callchain__printf(struct perf_evsel *evsel,
>> > +struct perf_sample *sample)
>> >  {
>> >unsigned int i;
>> > +  struct ip_callchain *callchain = sample->callchain;
>> > +  bool lbr = has_branch_callstack(evsel);
>> >
>> > -  printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
>> > +  if (lbr) {
>> > +  struct branch_stack *lbr_stack = sample->branch_stack;
>> > +  u64 kernel_callchain_nr = callchain->nr;
>> >
>> > -  for (i = 0; i < sample->callchain->nr; i++)
>> > +  for (i = 0; i < kernel_callchain_nr; i++) {
>> > +  if (callchain->ips[i] == PERF_CONTEXT_USER)
>> > +  break;
>> > +  }
>> > +
>> > +  if ((i != kernel_callchain_nr) && lbr_stack->nr) {
>> > +  u64 total_nr;
>> > +  /*
>> > +   * LBR callstack can only get user call chain,
>> > +   * i is kernel call chain number,
>> > +   * 1 is PERF_CONTEXT_USER.
>> > +   *
>> > +   * The user call chain is stored in LBR registers.
>> > +   * LBR are pair registers. The caller is stored
>> > +   * in "from" register, while the callee is stored
>> > +   * in "to" register.
>> > +   * For example, there is a call stack
>> > +   * "A"->"B"->"C"->"D".
>> > +   * The LBR registers will recorde like
>> > +   * "C"->"D", "B"->"C", "A"->"B".
>> > +   * So only the first "to" register and all "from"
>> > +   * registers are needed to construct the whole
>> stack.
>> > +   */
>> 
>> Andi is using some sanity checks:
>> http://marc.info/?l=linux-kernel=141584447819894=2
>> I guess this could be applied in here, once his patch gets in.
>> 
>
> Are you suggesting me to remove the comments,
> or rebase the whole patch to Andi's patch once it's merged?
>
> The branch history in Andi's patch is different as the call stack,
> although they are both from LBR.
> Andi's branch history recording branch records for
> taken branches, interrupts, and exceptions.
> While the LBR call stack records for the call stack.

Right.  And branch history can overlap with normal callchains so
additional check in there is to remove duplication.  While LBR call
stack is separated to user only so there should be no overlap.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Mon, 17 Nov 2014 16:55:53 +0100, Jiri Olsa wrote:
> On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:
>
> SNIP
>
>> +if (lbr) {
>> +for (i = 0; i < chain_nr; i++) {
>> +if (chain->ips[i] == PERF_CONTEXT_USER)
>> +break;
>> +}
>> +
>> +/* LBR only affects the user callchain */
>> +if (i != chain_nr) {
>> +struct branch_stack *lbr_stack = sample->branch_stack;
>> +int lbr_nr = lbr_stack->nr;
>> +/*
>> + * LBR callstack can only get user call chain.
>> + * The mix_chain_nr is kernel call chain
>> + * number plus LBR user call chain number.
>> + * i is kernel call chain number,
>> + * 1 is PERF_CONTEXT_USER,
>> + * lbr_nr + 1 is the user call chain number.
>> + * For details, please refer to the comments
>> + * in callchain__printf
>> + */
>> +int mix_chain_nr = i + 1 + lbr_nr + 1;
>> +
>> +if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
>> +pr_warning("corrupted callchain. 
>> skipping...\n");
>> +return 0;
>> +}
>> +
>> +for (j = 0; j < mix_chain_nr; j++) {
>> +if (callchain_param.order == ORDER_CALLEE) {
>> +if (j < i + 1)
>> +ip = chain->ips[j];
>> +else if (j > i + 1)
>> +ip = lbr_stack->entries[j - i - 
>> 2].from;
>> +else
>> +ip = lbr_stack->entries[0].to;
>> +} else {
>> +if (j < lbr_nr)
>> +ip = lbr_stack->entries[lbr_nr 
>> - j - 1].from;
>> +else if (j > lbr_nr)
>> +ip = chain->ips[i + 1 - (j - 
>> lbr_nr)];
>> +else
>> +ip = lbr_stack->entries[0].to;
>> +}
>> +
>> +err = add_callchain_ip(thread, parent, root_al,
>> +   cpumode, ip);
>> +if (err)
>> +goto exit;
>> +}
>> +return 0;
>> +}
>> +}
>
> also could you please move whole block above into separated function?
> Andi has another change for this function and it's becoming really big.

Agreed.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Liang, Kan


> SNIP
> 
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index f4478ce..335c3a9 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -557,15 +557,63 @@ int perf_session_queue_event(struct
> perf_session *s, union perf_event *event,
> > return 0;
> >  }
> >
> > -static void callchain__printf(struct perf_sample *sample)
> > +static void callchain__printf(struct perf_evsel *evsel,
> > + struct perf_sample *sample)
> >  {
> > unsigned int i;
> > +   struct ip_callchain *callchain = sample->callchain;
> > +   bool lbr = has_branch_callstack(evsel);
> >
> > -   printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
> > +   if (lbr) {
> > +   struct branch_stack *lbr_stack = sample->branch_stack;
> > +   u64 kernel_callchain_nr = callchain->nr;
> >
> > -   for (i = 0; i < sample->callchain->nr; i++)
> > +   for (i = 0; i < kernel_callchain_nr; i++) {
> > +   if (callchain->ips[i] == PERF_CONTEXT_USER)
> > +   break;
> > +   }
> > +
> > +   if ((i != kernel_callchain_nr) && lbr_stack->nr) {
> > +   u64 total_nr;
> > +   /*
> > +* LBR callstack can only get user call chain,
> > +* i is kernel call chain number,
> > +* 1 is PERF_CONTEXT_USER.
> > +*
> > +* The user call chain is stored in LBR registers.
> > +* LBR are pair registers. The caller is stored
> > +* in "from" register, while the callee is stored
> > +* in "to" register.
> > +* For example, there is a call stack
> > +* "A"->"B"->"C"->"D".
> > +* The LBR registers will recorde like
> > +* "C"->"D", "B"->"C", "A"->"B".
> > +* So only the first "to" register and all "from"
> > +* registers are needed to construct the whole
> stack.
> > +*/
> 
> Andi is using some sanity checks:
> http://marc.info/?l=linux-kernel=141584447819894=2
> I guess this could be applied in here, once his patch gets in.
> 

Are you suggesting me to remove the comments,
or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack,
although they are both from LBR.
Andi's branch history recording branch records for
taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Thanks,
Kan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:

SNIP

> + if (lbr) {
> + for (i = 0; i < chain_nr; i++) {
> + if (chain->ips[i] == PERF_CONTEXT_USER)
> + break;
> + }
> +
> + /* LBR only affects the user callchain */
> + if (i != chain_nr) {
> + struct branch_stack *lbr_stack = sample->branch_stack;
> + int lbr_nr = lbr_stack->nr;
> + /*
> +  * LBR callstack can only get user call chain.
> +  * The mix_chain_nr is kernel call chain
> +  * number plus LBR user call chain number.
> +  * i is kernel call chain number,
> +  * 1 is PERF_CONTEXT_USER,
> +  * lbr_nr + 1 is the user call chain number.
> +  * For details, please refer to the comments
> +  * in callchain__printf
> +  */
> + int mix_chain_nr = i + 1 + lbr_nr + 1;
> +
> + if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
> + pr_warning("corrupted callchain. 
> skipping...\n");
> + return 0;
> + }
> +
> + for (j = 0; j < mix_chain_nr; j++) {
> + if (callchain_param.order == ORDER_CALLEE) {
> + if (j < i + 1)
> + ip = chain->ips[j];
> + else if (j > i + 1)
> + ip = lbr_stack->entries[j - i - 
> 2].from;
> + else
> + ip = lbr_stack->entries[0].to;
> + } else {
> + if (j < lbr_nr)
> + ip = lbr_stack->entries[lbr_nr 
> - j - 1].from;
> + else if (j > lbr_nr)
> + ip = chain->ips[i + 1 - (j - 
> lbr_nr)];
> + else
> + ip = lbr_stack->entries[0].to;
> + }
> +
> + err = add_callchain_ip(thread, parent, root_al,
> +cpumode, ip);
> + if (err)
> + goto exit;
> + }
> + return 0;
> + }
> + }

also could you please move whole block above into separated function?
Andi has another change for this function and it's becoming really big.

other then that I think it's ok IMO and I could ack next version

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:

SNIP

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index f4478ce..335c3a9 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -557,15 +557,63 @@ int perf_session_queue_event(struct perf_session *s, 
> union perf_event *event,
>   return 0;
>  }
>  
> -static void callchain__printf(struct perf_sample *sample)
> +static void callchain__printf(struct perf_evsel *evsel,
> +   struct perf_sample *sample)
>  {
>   unsigned int i;
> + struct ip_callchain *callchain = sample->callchain;
> + bool lbr = has_branch_callstack(evsel);
>  
> - printf("... chain: nr:%" PRIu64 "\n", sample->callchain->nr);
> + if (lbr) {
> + struct branch_stack *lbr_stack = sample->branch_stack;
> + u64 kernel_callchain_nr = callchain->nr;
>  
> - for (i = 0; i < sample->callchain->nr; i++)
> + for (i = 0; i < kernel_callchain_nr; i++) {
> + if (callchain->ips[i] == PERF_CONTEXT_USER)
> + break;
> + }
> +
> + if ((i != kernel_callchain_nr) && lbr_stack->nr) {
> + u64 total_nr;
> + /*
> +  * LBR callstack can only get user call chain,
> +  * i is kernel call chain number,
> +  * 1 is PERF_CONTEXT_USER.
> +  *
> +  * The user call chain is stored in LBR registers.
> +  * LBR are pair registers. The caller is stored
> +  * in "from" register, while the callee is stored
> +  * in "to" register.
> +  * For example, there is a call stack
> +  * "A"->"B"->"C"->"D".
> +  * The LBR registers will recorde like
> +  * "C"->"D", "B"->"C", "A"->"B".
> +  * So only the first "to" register and all "from"
> +  * registers are needed to construct the whole stack.
> +  */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernel=141584447819894=2
I guess this could be applied in here, once his patch gets in.

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:

SNIP

 diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
 index f4478ce..335c3a9 100644
 --- a/tools/perf/util/session.c
 +++ b/tools/perf/util/session.c
 @@ -557,15 +557,63 @@ int perf_session_queue_event(struct perf_session *s, 
 union perf_event *event,
   return 0;
  }
  
 -static void callchain__printf(struct perf_sample *sample)
 +static void callchain__printf(struct perf_evsel *evsel,
 +   struct perf_sample *sample)
  {
   unsigned int i;
 + struct ip_callchain *callchain = sample-callchain;
 + bool lbr = has_branch_callstack(evsel);
  
 - printf(... chain: nr:% PRIu64 \n, sample-callchain-nr);
 + if (lbr) {
 + struct branch_stack *lbr_stack = sample-branch_stack;
 + u64 kernel_callchain_nr = callchain-nr;
  
 - for (i = 0; i  sample-callchain-nr; i++)
 + for (i = 0; i  kernel_callchain_nr; i++) {
 + if (callchain-ips[i] == PERF_CONTEXT_USER)
 + break;
 + }
 +
 + if ((i != kernel_callchain_nr)  lbr_stack-nr) {
 + u64 total_nr;
 + /*
 +  * LBR callstack can only get user call chain,
 +  * i is kernel call chain number,
 +  * 1 is PERF_CONTEXT_USER.
 +  *
 +  * The user call chain is stored in LBR registers.
 +  * LBR are pair registers. The caller is stored
 +  * in from register, while the callee is stored
 +  * in to register.
 +  * For example, there is a call stack
 +  * A-B-C-D.
 +  * The LBR registers will recorde like
 +  * C-D, B-C, A-B.
 +  * So only the first to register and all from
 +  * registers are needed to construct the whole stack.
 +  */

Andi is using some sanity checks:
http://marc.info/?l=linux-kernelm=141584447819894w=2
I guess this could be applied in here, once his patch gets in.

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:

SNIP

 + if (lbr) {
 + for (i = 0; i  chain_nr; i++) {
 + if (chain-ips[i] == PERF_CONTEXT_USER)
 + break;
 + }
 +
 + /* LBR only affects the user callchain */
 + if (i != chain_nr) {
 + struct branch_stack *lbr_stack = sample-branch_stack;
 + int lbr_nr = lbr_stack-nr;
 + /*
 +  * LBR callstack can only get user call chain.
 +  * The mix_chain_nr is kernel call chain
 +  * number plus LBR user call chain number.
 +  * i is kernel call chain number,
 +  * 1 is PERF_CONTEXT_USER,
 +  * lbr_nr + 1 is the user call chain number.
 +  * For details, please refer to the comments
 +  * in callchain__printf
 +  */
 + int mix_chain_nr = i + 1 + lbr_nr + 1;
 +
 + if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
 + pr_warning(corrupted callchain. 
 skipping...\n);
 + return 0;
 + }
 +
 + for (j = 0; j  mix_chain_nr; j++) {
 + if (callchain_param.order == ORDER_CALLEE) {
 + if (j  i + 1)
 + ip = chain-ips[j];
 + else if (j  i + 1)
 + ip = lbr_stack-entries[j - i - 
 2].from;
 + else
 + ip = lbr_stack-entries[0].to;
 + } else {
 + if (j  lbr_nr)
 + ip = lbr_stack-entries[lbr_nr 
 - j - 1].from;
 + else if (j  lbr_nr)
 + ip = chain-ips[i + 1 - (j - 
 lbr_nr)];
 + else
 + ip = lbr_stack-entries[0].to;
 + }
 +
 + err = add_callchain_ip(thread, parent, root_al,
 +cpumode, ip);
 + if (err)
 + goto exit;
 + }
 + return 0;
 + }
 + }

also could you please move whole block above into separated function?
Andi has another change for this function and it's becoming really big.

other then that I think it's ok IMO and I could ack next version

thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Liang, Kan


 SNIP
 
  diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
  index f4478ce..335c3a9 100644
  --- a/tools/perf/util/session.c
  +++ b/tools/perf/util/session.c
  @@ -557,15 +557,63 @@ int perf_session_queue_event(struct
 perf_session *s, union perf_event *event,
  return 0;
   }
 
  -static void callchain__printf(struct perf_sample *sample)
  +static void callchain__printf(struct perf_evsel *evsel,
  + struct perf_sample *sample)
   {
  unsigned int i;
  +   struct ip_callchain *callchain = sample-callchain;
  +   bool lbr = has_branch_callstack(evsel);
 
  -   printf(... chain: nr:% PRIu64 \n, sample-callchain-nr);
  +   if (lbr) {
  +   struct branch_stack *lbr_stack = sample-branch_stack;
  +   u64 kernel_callchain_nr = callchain-nr;
 
  -   for (i = 0; i  sample-callchain-nr; i++)
  +   for (i = 0; i  kernel_callchain_nr; i++) {
  +   if (callchain-ips[i] == PERF_CONTEXT_USER)
  +   break;
  +   }
  +
  +   if ((i != kernel_callchain_nr)  lbr_stack-nr) {
  +   u64 total_nr;
  +   /*
  +* LBR callstack can only get user call chain,
  +* i is kernel call chain number,
  +* 1 is PERF_CONTEXT_USER.
  +*
  +* The user call chain is stored in LBR registers.
  +* LBR are pair registers. The caller is stored
  +* in from register, while the callee is stored
  +* in to register.
  +* For example, there is a call stack
  +* A-B-C-D.
  +* The LBR registers will recorde like
  +* C-D, B-C, A-B.
  +* So only the first to register and all from
  +* registers are needed to construct the whole
 stack.
  +*/
 
 Andi is using some sanity checks:
 http://marc.info/?l=linux-kernelm=141584447819894w=2
 I guess this could be applied in here, once his patch gets in.
 

Are you suggesting me to remove the comments,
or rebase the whole patch to Andi's patch once it's merged?

The branch history in Andi's patch is different as the call stack,
although they are both from LBR.
Andi's branch history recording branch records for
taken branches, interrupts, and exceptions.
While the LBR call stack records for the call stack.

Thanks,
Kan

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Mon, 17 Nov 2014 17:41:32 +, Kan Liang wrote:
 SNIP
 
  diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
  index f4478ce..335c3a9 100644
  --- a/tools/perf/util/session.c
  +++ b/tools/perf/util/session.c
  @@ -557,15 +557,63 @@ int perf_session_queue_event(struct
 perf_session *s, union perf_event *event,
 return 0;
   }
 
  -static void callchain__printf(struct perf_sample *sample)
  +static void callchain__printf(struct perf_evsel *evsel,
  +struct perf_sample *sample)
   {
 unsigned int i;
  +  struct ip_callchain *callchain = sample-callchain;
  +  bool lbr = has_branch_callstack(evsel);
 
  -  printf(... chain: nr:% PRIu64 \n, sample-callchain-nr);
  +  if (lbr) {
  +  struct branch_stack *lbr_stack = sample-branch_stack;
  +  u64 kernel_callchain_nr = callchain-nr;
 
  -  for (i = 0; i  sample-callchain-nr; i++)
  +  for (i = 0; i  kernel_callchain_nr; i++) {
  +  if (callchain-ips[i] == PERF_CONTEXT_USER)
  +  break;
  +  }
  +
  +  if ((i != kernel_callchain_nr)  lbr_stack-nr) {
  +  u64 total_nr;
  +  /*
  +   * LBR callstack can only get user call chain,
  +   * i is kernel call chain number,
  +   * 1 is PERF_CONTEXT_USER.
  +   *
  +   * The user call chain is stored in LBR registers.
  +   * LBR are pair registers. The caller is stored
  +   * in from register, while the callee is stored
  +   * in to register.
  +   * For example, there is a call stack
  +   * A-B-C-D.
  +   * The LBR registers will recorde like
  +   * C-D, B-C, A-B.
  +   * So only the first to register and all from
  +   * registers are needed to construct the whole
 stack.
  +   */
 
 Andi is using some sanity checks:
 http://marc.info/?l=linux-kernelm=141584447819894w=2
 I guess this could be applied in here, once his patch gets in.
 

 Are you suggesting me to remove the comments,
 or rebase the whole patch to Andi's patch once it's merged?

 The branch history in Andi's patch is different as the call stack,
 although they are both from LBR.
 Andi's branch history recording branch records for
 taken branches, interrupts, and exceptions.
 While the LBR call stack records for the call stack.

Right.  And branch history can overlap with normal callchains so
additional check in there is to remove duplication.  While LBR call
stack is separated to user only so there should be no overlap.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Mon, 17 Nov 2014 16:55:53 +0100, Jiri Olsa wrote:
 On Fri, Nov 14, 2014 at 08:44:12AM -0500, kan.li...@intel.com wrote:

 SNIP

 +if (lbr) {
 +for (i = 0; i  chain_nr; i++) {
 +if (chain-ips[i] == PERF_CONTEXT_USER)
 +break;
 +}
 +
 +/* LBR only affects the user callchain */
 +if (i != chain_nr) {
 +struct branch_stack *lbr_stack = sample-branch_stack;
 +int lbr_nr = lbr_stack-nr;
 +/*
 + * LBR callstack can only get user call chain.
 + * The mix_chain_nr is kernel call chain
 + * number plus LBR user call chain number.
 + * i is kernel call chain number,
 + * 1 is PERF_CONTEXT_USER,
 + * lbr_nr + 1 is the user call chain number.
 + * For details, please refer to the comments
 + * in callchain__printf
 + */
 +int mix_chain_nr = i + 1 + lbr_nr + 1;
 +
 +if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
 +pr_warning(corrupted callchain. 
 skipping...\n);
 +return 0;
 +}
 +
 +for (j = 0; j  mix_chain_nr; j++) {
 +if (callchain_param.order == ORDER_CALLEE) {
 +if (j  i + 1)
 +ip = chain-ips[j];
 +else if (j  i + 1)
 +ip = lbr_stack-entries[j - i - 
 2].from;
 +else
 +ip = lbr_stack-entries[0].to;
 +} else {
 +if (j  lbr_nr)
 +ip = lbr_stack-entries[lbr_nr 
 - j - 1].from;
 +else if (j  lbr_nr)
 +ip = chain-ips[i + 1 - (j - 
 lbr_nr)];
 +else
 +ip = lbr_stack-entries[0].to;
 +}
 +
 +err = add_callchain_ip(thread, parent, root_al,
 +   cpumode, ip);
 +if (err)
 +goto exit;
 +}
 +return 0;
 +}
 +}

 also could you please move whole block above into separated function?
 Andi has another change for this function and it's becoming really big.

Agreed.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Namhyung Kim
On Fri, 14 Nov 2014 08:44:12 -0500, kan liang wrote:
 + /* LBR only affects the user callchain */
 + if (i != chain_nr) {
 + struct branch_stack *lbr_stack = sample-branch_stack;
 + int lbr_nr = lbr_stack-nr;
 + /*
 +  * LBR callstack can only get user call chain.
 +  * The mix_chain_nr is kernel call chain
 +  * number plus LBR user call chain number.
 +  * i is kernel call chain number,
 +  * 1 is PERF_CONTEXT_USER,
 +  * lbr_nr + 1 is the user call chain number.
 +  * For details, please refer to the comments
 +  * in callchain__printf
 +  */
 + int mix_chain_nr = i + 1 + lbr_nr + 1;
 +
 + if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
 + pr_warning(corrupted callchain. 
 skipping...\n);
 + return 0;
 + }

I'm not sure whether this is really a corrupted callchain.  If a single
chain is greater than the max it should be corrupted, but we're now
summing up with other values..

What about checking callchain_nr and lbr_nr separately or mix_nr with
2*max?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-17 Thread Jiri Olsa
On Tue, Nov 18, 2014 at 03:13:50PM +0900, Namhyung Kim wrote:

SNIP

   + * in from register, while the callee is 
   stored
   + * in to register.
   + * For example, there is a call stack
   + * A-B-C-D.
   + * The LBR registers will recorde like
   + * C-D, B-C, A-B.
   + * So only the first to register and all 
   from
   + * registers are needed to construct the whole
  stack.
   + */
  
  Andi is using some sanity checks:
  http://marc.info/?l=linux-kernelm=141584447819894w=2
  I guess this could be applied in here, once his patch gets in.
  
 
  Are you suggesting me to remove the comments,
  or rebase the whole patch to Andi's patch once it's merged?
 
  The branch history in Andi's patch is different as the call stack,
  although they are both from LBR.
  Andi's branch history recording branch records for
  taken branches, interrupts, and exceptions.
  While the LBR call stack records for the call stack.
 
 Right.  And branch history can overlap with normal callchains so
 additional check in there is to remove duplication.  While LBR call
 stack is separated to user only so there should be no overlap.

hum, it seemed to me like the remove_loops function could be used
for this one as well.. but anyway I meant that this can be introduced
later after Andi's change gets in

thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-14 Thread kan . liang
From: Kan Liang 

LBR call stack only has user callchain. It is output as
PERF_SAMPLE_BRANCH_STACK data format. For the kernel callchain, it's
still from PERF_SAMPLE_CALLCHAIN.
The perf tool has to handle both data sources to construct a
complete callstack.
For perf report -D option, both lbr and fp information will be
displayed.

Signed-off-by: Kan Liang 
---
 tools/perf/util/evsel.h   |  4 +++
 tools/perf/util/machine.c | 72 +--
 tools/perf/util/session.c | 60 +++
 3 files changed, 122 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 9797909..1bbaa74 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -368,4 +368,8 @@ for ((_evsel) = list_entry((_leader)->node.next, struct 
perf_evsel, node);  \
  (_evsel) && (_evsel)->leader == (_leader);
\
  (_evsel) = list_entry((_evsel)->node.next, struct perf_evsel, node))
 
+static inline bool has_branch_callstack(struct perf_evsel *evsel)
+{
+   return evsel->attr.branch_sample_type & PERF_SAMPLE_BRANCH_CALL_STACK;
+}
 #endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index dd8496a..a26c9e0 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1452,15 +1452,19 @@ struct branch_info *sample__resolve_bstack(struct 
perf_sample *sample,
 }
 
 static int thread__resolve_callchain_sample(struct thread *thread,
-struct ip_callchain *chain,
-struct symbol **parent,
-struct addr_location *root_al,
-int max_stack)
+   struct perf_evsel *evsel,
+   struct perf_sample *sample,
+   struct symbol **parent,
+   struct addr_location *root_al,
+   int max_stack)
 {
+   struct ip_callchain *chain = sample->callchain;
u8 cpumode = PERF_RECORD_MISC_USER;
int chain_nr = min(max_stack, (int)chain->nr);
int i, j, err = 0;
int skip_idx __maybe_unused;
+   u64 ip;
+   bool lbr = has_branch_callstack(evsel);
 
callchain_cursor_reset(_cursor);
 
@@ -1469,6 +1473,59 @@ static int thread__resolve_callchain_sample(struct 
thread *thread,
return 0;
}
 
+   if (lbr) {
+   for (i = 0; i < chain_nr; i++) {
+   if (chain->ips[i] == PERF_CONTEXT_USER)
+   break;
+   }
+
+   /* LBR only affects the user callchain */
+   if (i != chain_nr) {
+   struct branch_stack *lbr_stack = sample->branch_stack;
+   int lbr_nr = lbr_stack->nr;
+   /*
+* LBR callstack can only get user call chain.
+* The mix_chain_nr is kernel call chain
+* number plus LBR user call chain number.
+* i is kernel call chain number,
+* 1 is PERF_CONTEXT_USER,
+* lbr_nr + 1 is the user call chain number.
+* For details, please refer to the comments
+* in callchain__printf
+*/
+   int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+   if (mix_chain_nr > PERF_MAX_STACK_DEPTH) {
+   pr_warning("corrupted callchain. 
skipping...\n");
+   return 0;
+   }
+
+   for (j = 0; j < mix_chain_nr; j++) {
+   if (callchain_param.order == ORDER_CALLEE) {
+   if (j < i + 1)
+   ip = chain->ips[j];
+   else if (j > i + 1)
+   ip = lbr_stack->entries[j - i - 
2].from;
+   else
+   ip = lbr_stack->entries[0].to;
+   } else {
+   if (j < lbr_nr)
+   ip = lbr_stack->entries[lbr_nr 
- j - 1].from;
+   else if (j > lbr_nr)
+   ip = chain->ips[i + 1 - (j - 
lbr_nr)];
+   else
+   ip = lbr_stack->entries[0].to;
+   }
+
+   err = add_callchain_ip(thread, parent, root_al,
+ 

[PATCH V3 3/3] perf tools: Construct LBR call chain

2014-11-14 Thread kan . liang
From: Kan Liang kan.li...@intel.com

LBR call stack only has user callchain. It is output as
PERF_SAMPLE_BRANCH_STACK data format. For the kernel callchain, it's
still from PERF_SAMPLE_CALLCHAIN.
The perf tool has to handle both data sources to construct a
complete callstack.
For perf report -D option, both lbr and fp information will be
displayed.

Signed-off-by: Kan Liang kan.li...@intel.com
---
 tools/perf/util/evsel.h   |  4 +++
 tools/perf/util/machine.c | 72 +--
 tools/perf/util/session.c | 60 +++
 3 files changed, 122 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 9797909..1bbaa74 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -368,4 +368,8 @@ for ((_evsel) = list_entry((_leader)-node.next, struct 
perf_evsel, node);  \
  (_evsel)  (_evsel)-leader == (_leader);
\
  (_evsel) = list_entry((_evsel)-node.next, struct perf_evsel, node))
 
+static inline bool has_branch_callstack(struct perf_evsel *evsel)
+{
+   return evsel-attr.branch_sample_type  PERF_SAMPLE_BRANCH_CALL_STACK;
+}
 #endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index dd8496a..a26c9e0 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1452,15 +1452,19 @@ struct branch_info *sample__resolve_bstack(struct 
perf_sample *sample,
 }
 
 static int thread__resolve_callchain_sample(struct thread *thread,
-struct ip_callchain *chain,
-struct symbol **parent,
-struct addr_location *root_al,
-int max_stack)
+   struct perf_evsel *evsel,
+   struct perf_sample *sample,
+   struct symbol **parent,
+   struct addr_location *root_al,
+   int max_stack)
 {
+   struct ip_callchain *chain = sample-callchain;
u8 cpumode = PERF_RECORD_MISC_USER;
int chain_nr = min(max_stack, (int)chain-nr);
int i, j, err = 0;
int skip_idx __maybe_unused;
+   u64 ip;
+   bool lbr = has_branch_callstack(evsel);
 
callchain_cursor_reset(callchain_cursor);
 
@@ -1469,6 +1473,59 @@ static int thread__resolve_callchain_sample(struct 
thread *thread,
return 0;
}
 
+   if (lbr) {
+   for (i = 0; i  chain_nr; i++) {
+   if (chain-ips[i] == PERF_CONTEXT_USER)
+   break;
+   }
+
+   /* LBR only affects the user callchain */
+   if (i != chain_nr) {
+   struct branch_stack *lbr_stack = sample-branch_stack;
+   int lbr_nr = lbr_stack-nr;
+   /*
+* LBR callstack can only get user call chain.
+* The mix_chain_nr is kernel call chain
+* number plus LBR user call chain number.
+* i is kernel call chain number,
+* 1 is PERF_CONTEXT_USER,
+* lbr_nr + 1 is the user call chain number.
+* For details, please refer to the comments
+* in callchain__printf
+*/
+   int mix_chain_nr = i + 1 + lbr_nr + 1;
+
+   if (mix_chain_nr  PERF_MAX_STACK_DEPTH) {
+   pr_warning(corrupted callchain. 
skipping...\n);
+   return 0;
+   }
+
+   for (j = 0; j  mix_chain_nr; j++) {
+   if (callchain_param.order == ORDER_CALLEE) {
+   if (j  i + 1)
+   ip = chain-ips[j];
+   else if (j  i + 1)
+   ip = lbr_stack-entries[j - i - 
2].from;
+   else
+   ip = lbr_stack-entries[0].to;
+   } else {
+   if (j  lbr_nr)
+   ip = lbr_stack-entries[lbr_nr 
- j - 1].from;
+   else if (j  lbr_nr)
+   ip = chain-ips[i + 1 - (j - 
lbr_nr)];
+   else
+   ip = lbr_stack-entries[0].to;
+   }
+
+   err = add_callchain_ip(thread,