linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf hists browser: Fix UI bug after fold/unfold
@ 2015-03-06 12:51 He Kuang
  2015-03-10  6:28 ` Namhyung Kim
  0 siblings, 1 reply; 14+ messages in thread
From: He Kuang @ 2015-03-06 12:51 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme, namhyung, jolsa
  Cc: wangnan0, yunlong.song, linux-kernel

In perf hists browser, the fold/unfold stat of each hist entry is
recorded but hb->nr_callchain_rows loses its value after zoom out and
zoom in back. This causes a wrong row cursor range that restrict user to
move down anymore.

Before:
  $ perf record -g -e syscalls:* ls
  $ perf report

(select an event and unfold, then zoom out and back, row cursor blocked)

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       16.67% 0x692d6f732e6f7364
       8.33% 0x442d00746f6f725f
       8.33% 0x746f006469
       8.33% 0x646c6f2e617461
       8.33% 0x646970
       8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
       8.33% 0x7365
       8.33% 0x6469
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64

When zoom into DSO or thread, nr_rows is not cleared which causes a
similar problem.

This patch recalculates hb->nr_callchain_rows and clears nr_rows to fix
the bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
 tools/perf/util/hist.c         |  1 +
 2 files changed, 21 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1106bb8..bef9ab0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static struct rb_node *hists__filter_entries(struct rb_node *nd,
 					     float min_pcnt);
+static int __hist_browser__get_folding(struct hist_browser *browser);
 
 static bool hist_browser__has_filter(struct hist_browser *hb)
 {
@@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
 	else
 		nr_entries = hb->hists->nr_entries;
 
+	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
 	return nr_entries + hb->nr_callchain_rows;
 }
 
@@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
 		he->nr_rows = 0;
 }
 
+static int
+__hist_browser__get_folding(struct hist_browser *browser)
+{
+	struct rb_node *nd;
+	struct hists *hists = browser->hists;
+	int unfolded_rows = 0;
+
+	for (nd = rb_first(&hists->entries);
+	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
+	     nd = rb_next(nd)) {
+		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
+
+		if (he->ms.unfolded)
+			unfolded_rows += he->nr_rows;
+	}
+	return unfolded_rows;
+}
+
 static void
 __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
 {
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 70b48a6..24aff7d 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	/* force fold unfiltered entry for simplicity */
 	h->ms.unfolded = false;
 	h->row_offset = 0;
+	h->nr_rows = 0;
 
 	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
 
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf hists browser: Fix UI bug after fold/unfold
  2015-03-06 12:51 [PATCH] perf hists browser: Fix UI bug after fold/unfold He Kuang
@ 2015-03-10  6:28 ` Namhyung Kim
  2015-03-10  7:26   ` He Kuang
  0 siblings, 1 reply; 14+ messages in thread
From: Namhyung Kim @ 2015-03-10  6:28 UTC (permalink / raw)
  To: He Kuang
  Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, yunlong.song,
	linux-kernel

On Fri, Mar 06, 2015 at 08:51:44PM +0800, He Kuang wrote:
> In perf hists browser, the fold/unfold stat of each hist entry is
> recorded but hb->nr_callchain_rows loses its value after zoom out and
> zoom in back. This causes a wrong row cursor range that restrict user to
> move down anymore.
> 
> Before:
>   $ perf record -g -e syscalls:* ls
>   $ perf report
> 
> (select an event and unfold, then zoom out and back, row cursor blocked)
> 
>     Children      Self  Command  Shared Object          Symbol
>   ================================================================
>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>   - lstat64
>        16.67% 0x6469702e64
>        16.67% 0x692d6f732e6f7364
>        8.33% 0x442d00746f6f725f
>        8.33% 0x746f006469
>        8.33% 0x646c6f2e617461
>        8.33% 0x646970
>        8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
>        8.33% 0x7365
>        8.33% 0x6469
>        8.33% 0x65
>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>      0x6469702e64
> 
> When zoom into DSO or thread, nr_rows is not cleared which causes a
> similar problem.
> 
> This patch recalculates hb->nr_callchain_rows and clears nr_rows to fix
> the bug.

The intention was that it should reset the folding state when a filter
is applied.  So I guess just resetting he->nr_rows to 0 in hists__
remove_entry_filter() would solve the problem.

Thanks,
Namhyung


> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
>  tools/perf/util/hist.c         |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 1106bb8..bef9ab0 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
>  
>  static struct rb_node *hists__filter_entries(struct rb_node *nd,
>  					     float min_pcnt);
> +static int __hist_browser__get_folding(struct hist_browser *browser);
>  
>  static bool hist_browser__has_filter(struct hist_browser *hb)
>  {
> @@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
>  	else
>  		nr_entries = hb->hists->nr_entries;
>  
> +	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
>  	return nr_entries + hb->nr_callchain_rows;
>  }
>  
> @@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
>  		he->nr_rows = 0;
>  }
>  
> +static int
> +__hist_browser__get_folding(struct hist_browser *browser)
> +{
> +	struct rb_node *nd;
> +	struct hists *hists = browser->hists;
> +	int unfolded_rows = 0;
> +
> +	for (nd = rb_first(&hists->entries);
> +	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
> +	     nd = rb_next(nd)) {
> +		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
> +
> +		if (he->ms.unfolded)
> +			unfolded_rows += he->nr_rows;
> +	}
> +	return unfolded_rows;
> +}
> +
>  static void
>  __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
>  {
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 70b48a6..24aff7d 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
>  	/* force fold unfiltered entry for simplicity */
>  	h->ms.unfolded = false;
>  	h->row_offset = 0;
> +	h->nr_rows = 0;
>  
>  	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
>  
> -- 
> 2.2.0.33.gc18b867
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf hists browser: Fix UI bug after fold/unfold
  2015-03-10  6:28 ` Namhyung Kim
@ 2015-03-10  7:26   ` He Kuang
  2015-03-11  0:26     ` Namhyung Kim
  0 siblings, 1 reply; 14+ messages in thread
From: He Kuang @ 2015-03-10  7:26 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, yunlong.song,
	linux-kernel

hi,
On 2015/3/10 14:28, Namhyung Kim wrote:
> On Fri, Mar 06, 2015 at 08:51:44PM +0800, He Kuang wrote:
>> In perf hists browser, the fold/unfold stat of each hist entry is
>> recorded but hb->nr_callchain_rows loses its value after zoom out and
>> zoom in back. This causes a wrong row cursor range that restrict user to
>> move down anymore.
>>
>> Before:
>>    $ perf record -g -e syscalls:* ls
>>    $ perf report
>>
>> (select an event and unfold, then zoom out and back, row cursor blocked)
>>
>>      Children      Self  Command  Shared Object          Symbol
>>    ================================================================
>>    -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>>    - lstat64
>>         16.67% 0x6469702e64
>>         16.67% 0x692d6f732e6f7364
>>         8.33% 0x442d00746f6f725f
>>         8.33% 0x746f006469
>>         8.33% 0x646c6f2e617461
>>         8.33% 0x646970
>>         8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
>>         8.33% 0x7365
>>         8.33% 0x6469
>>         8.33% 0x65
>>    -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>>       0x6469702e64
>>
>> When zoom into DSO or thread, nr_rows is not cleared which causes a
>> similar problem.
>>
>> This patch recalculates hb->nr_callchain_rows and clears nr_rows to fix
>> the bug.
> The intention was that it should reset the folding state when a filter
> is applied.  So I guess just resetting he->nr_rows to 0 in hists__
> remove_entry_filter() would solve the problem.
>
> Thanks,
> Namhyung

Actually, this patch fixes two bugs. The first one is caused by
fold/unfold stat not matched with hb->nr_callchain_rows, which is
not related to hists__remove_entry_filter. The second one is
caused by he->nr_rows not cleared when zoom into DSO or thread.

These two bugs both have a same problem as I described in the
chat, so I thought it can be resloved in one patch. What do you
think?
>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
>>   tools/perf/util/hist.c         |  1 +
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index 1106bb8..bef9ab0 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
>>   
>>   static struct rb_node *hists__filter_entries(struct rb_node *nd,
>>   					     float min_pcnt);
>> +static int __hist_browser__get_folding(struct hist_browser *browser);
>>   
>>   static bool hist_browser__has_filter(struct hist_browser *hb)
>>   {
>> @@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
>>   	else
>>   		nr_entries = hb->hists->nr_entries;
>>   
>> +	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
>>   	return nr_entries + hb->nr_callchain_rows;
>>   }
>>   
>> @@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
>>   		he->nr_rows = 0;
>>   }
>>   
>> +static int
>> +__hist_browser__get_folding(struct hist_browser *browser)
>> +{
>> +	struct rb_node *nd;
>> +	struct hists *hists = browser->hists;
>> +	int unfolded_rows = 0;
>> +
>> +	for (nd = rb_first(&hists->entries);
>> +	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
>> +	     nd = rb_next(nd)) {
>> +		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
>> +
>> +		if (he->ms.unfolded)
>> +			unfolded_rows += he->nr_rows;
>> +	}
>> +	return unfolded_rows;
>> +}
>> +
>>   static void
>>   __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
>>   {
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index 70b48a6..24aff7d 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
>>   	/* force fold unfiltered entry for simplicity */
>>   	h->ms.unfolded = false;
>>   	h->row_offset = 0;
>> +	h->nr_rows = 0;
>>   
>>   	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
>>   
>> -- 
>> 2.2.0.33.gc18b867
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] perf hists browser: Fix UI bug after fold/unfold
  2015-03-10  7:26   ` He Kuang
@ 2015-03-11  0:26     ` Namhyung Kim
  2015-03-11 12:36       ` [PATCHv2 1/2] " He Kuang
  0 siblings, 1 reply; 14+ messages in thread
From: Namhyung Kim @ 2015-03-11  0:26 UTC (permalink / raw)
  To: He Kuang
  Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, yunlong.song,
	linux-kernel

Hi,

On Tue, Mar 10, 2015 at 03:26:41PM +0800, He Kuang wrote:
> hi,
> On 2015/3/10 14:28, Namhyung Kim wrote:
> >On Fri, Mar 06, 2015 at 08:51:44PM +0800, He Kuang wrote:
> >>In perf hists browser, the fold/unfold stat of each hist entry is
> >>recorded but hb->nr_callchain_rows loses its value after zoom out and
> >>zoom in back. This causes a wrong row cursor range that restrict user to
> >>move down anymore.
> >>
> >>Before:
> >>   $ perf record -g -e syscalls:* ls
> >>   $ perf report
> >>
> >>(select an event and unfold, then zoom out and back, row cursor blocked)
> >>
> >>     Children      Self  Command  Shared Object          Symbol
> >>   ================================================================
> >>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
> >>   - lstat64
> >>        16.67% 0x6469702e64
> >>        16.67% 0x692d6f732e6f7364
> >>        8.33% 0x442d00746f6f725f
> >>        8.33% 0x746f006469
> >>        8.33% 0x646c6f2e617461
> >>        8.33% 0x646970
> >>        8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
> >>        8.33% 0x7365
> >>        8.33% 0x6469
> >>        8.33% 0x65
> >>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
> >>      0x6469702e64
> >>
> >>When zoom into DSO or thread, nr_rows is not cleared which causes a
> >>similar problem.
> >>
> >>This patch recalculates hb->nr_callchain_rows and clears nr_rows to fix
> >>the bug.
> >The intention was that it should reset the folding state when a filter
> >is applied.  So I guess just resetting he->nr_rows to 0 in hists__
> >remove_entry_filter() would solve the problem.
> >
> >Thanks,
> >Namhyung
> 
> Actually, this patch fixes two bugs. The first one is caused by
> fold/unfold stat not matched with hb->nr_callchain_rows, which is
> not related to hists__remove_entry_filter.

So this is not related to zooming, right?  Could you please give me a
reproduce step then?


> The second one is
> caused by he->nr_rows not cleared when zoom into DSO or thread.

This will be solved by resetting he->nr_rows, I guess.


> 
> These two bugs both have a same problem as I described in the
> chat, so I thought it can be resloved in one patch. What do you
> think?

I think you'd better to split the fix into two patches with more
detailed description and/or reproduce steps.

Thanks,
Namhyung


> >
> >>Signed-off-by: He Kuang <hekuang@huawei.com>
> >>---
> >>  tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
> >>  tools/perf/util/hist.c         |  1 +
> >>  2 files changed, 21 insertions(+)
> >>
> >>diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> >>index 1106bb8..bef9ab0 100644
> >>--- a/tools/perf/ui/browsers/hists.c
> >>+++ b/tools/perf/ui/browsers/hists.c
> >>@@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
> >>  static struct rb_node *hists__filter_entries(struct rb_node *nd,
> >>  					     float min_pcnt);
> >>+static int __hist_browser__get_folding(struct hist_browser *browser);
> >>  static bool hist_browser__has_filter(struct hist_browser *hb)
> >>  {
> >>@@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
> >>  	else
> >>  		nr_entries = hb->hists->nr_entries;
> >>+	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
> >>  	return nr_entries + hb->nr_callchain_rows;
> >>  }
> >>@@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
> >>  		he->nr_rows = 0;
> >>  }
> >>+static int
> >>+__hist_browser__get_folding(struct hist_browser *browser)
> >>+{
> >>+	struct rb_node *nd;
> >>+	struct hists *hists = browser->hists;
> >>+	int unfolded_rows = 0;
> >>+
> >>+	for (nd = rb_first(&hists->entries);
> >>+	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
> >>+	     nd = rb_next(nd)) {
> >>+		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
> >>+
> >>+		if (he->ms.unfolded)
> >>+			unfolded_rows += he->nr_rows;
> >>+	}
> >>+	return unfolded_rows;
> >>+}
> >>+
> >>  static void
> >>  __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
> >>  {
> >>diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> >>index 70b48a6..24aff7d 100644
> >>--- a/tools/perf/util/hist.c
> >>+++ b/tools/perf/util/hist.c
> >>@@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
> >>  	/* force fold unfiltered entry for simplicity */
> >>  	h->ms.unfolded = false;
> >>  	h->row_offset = 0;
> >>+	h->nr_rows = 0;
> >>  	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
> >>-- 
> >>2.2.0.33.gc18b867
> >>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>Please read the FAQ at  http://www.tux.org/lkml/
> >
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold
  2015-03-11  0:26     ` Namhyung Kim
@ 2015-03-11 12:36       ` He Kuang
  2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
  2015-03-11 13:48         ` [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 14+ messages in thread
From: He Kuang @ 2015-03-11 12:36 UTC (permalink / raw)
  To: namhyung; +Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, linux-kernel

In perf hists browser, the fold/unfold stat of each hist entry is
recorded but hb->nr_callchain_rows loses its value after zoom out and
zoom in back. This causes a wrong row cursor range that restrict user to
move down anymore.

This bug can be reproduced as follows:

  $ perf record -g -e syscalls:* ls
  $ perf report

    Available samples
  ================================================================
    2 syscalls:sys_enter_mprotect <= [enter one of the entries]
    2 syscalls:sys_exit_mprotect
    13 syscalls:sys_enter_brk
    ...

In the hists brower, unfold some of the items, now the cursor can reach
to any rows:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64 <= [cursor can reach to bottom line, everything is ok]

Now, zoom back to "Available samples" and enter again:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64

This patch recalculates hb->nr_callchain_rows to fix the bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1106bb8..bef9ab0 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static struct rb_node *hists__filter_entries(struct rb_node *nd,
 					     float min_pcnt);
+static int __hist_browser__get_folding(struct hist_browser *browser);
 
 static bool hist_browser__has_filter(struct hist_browser *hb)
 {
@@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
 	else
 		nr_entries = hb->hists->nr_entries;
 
+	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
 	return nr_entries + hb->nr_callchain_rows;
 }
 
@@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
 		he->nr_rows = 0;
 }
 
+static int
+__hist_browser__get_folding(struct hist_browser *browser)
+{
+	struct rb_node *nd;
+	struct hists *hists = browser->hists;
+	int unfolded_rows = 0;
+
+	for (nd = rb_first(&hists->entries);
+	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
+	     nd = rb_next(nd)) {
+		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
+
+		if (he->ms.unfolded)
+			unfolded_rows += he->nr_rows;
+	}
+	return unfolded_rows;
+}
+
 static void
 __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
 {
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol
  2015-03-11 12:36       ` [PATCHv2 1/2] " He Kuang
@ 2015-03-11 12:36         ` He Kuang
  2015-03-11 14:12           ` Arnaldo Carvalho de Melo
                             ` (2 more replies)
  2015-03-11 13:48         ` [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold Arnaldo Carvalho de Melo
  1 sibling, 3 replies; 14+ messages in thread
From: He Kuang @ 2015-03-11 12:36 UTC (permalink / raw)
  To: namhyung; +Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, linux-kernel

When zoom into thread/dso/symbol, the fold/unfold stat is cleared in
hists__filter_by_thread/dso/symbol(), but h->nr_rows is not cleared. So
if we toggle fold stat on the unfold entires, nr_entries got a wrong
value.

This bug can be reproduced as follows:

$ perf record -g -e syscalls:sys_enter_open ls
$ perf report

    Children      Self  Command  Shared Object            Symbol
  ================================================================
  +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
  -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
      _dl_load_shared_library <= [Zoom into thread/dso]
      _dl_get_ready_to_run
      _start
  ...

In the new thread hists, all entries reset to fold, if we unfold the
same entry as we previously unfolded, nr_entries got wrong value, and we
can't move down cursor to bottom row.

                                                         Thread: ls
    Children      Self  Command  Shared Object            Symbol
  ================================================================
  +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
  -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
      _dl_load_shared_library
      _dl_get_ready_to_run <= [cursor may stop here, can't move down]
      _start
  ...

This patch clear h->nr_rows to fix this bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 70b48a6..24aff7d 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	/* force fold unfiltered entry for simplicity */
 	h->ms.unfolded = false;
 	h->row_offset = 0;
+	h->nr_rows = 0;
 
 	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
 
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold
  2015-03-11 12:36       ` [PATCHv2 1/2] " He Kuang
  2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
@ 2015-03-11 13:48         ` Arnaldo Carvalho de Melo
  2015-03-12  7:21           ` [PATCHv3] " He Kuang
  1 sibling, 1 reply; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-03-11 13:48 UTC (permalink / raw)
  To: He Kuang
  Cc: namhyung, a.p.zijlstra, paulus, mingo, jolsa, wangnan0, linux-kernel

Em Wed, Mar 11, 2015 at 08:36:02PM +0800, He Kuang escreveu:
> In perf hists browser, the fold/unfold stat of each hist entry is
> recorded but hb->nr_callchain_rows loses its value after zoom out and
> zoom in back. This causes a wrong row cursor range that restrict user to
> move down anymore.
> 
> This bug can be reproduced as follows:
> 
>   $ perf record -g -e syscalls:* ls
>   $ perf report
> 
>     Available samples
>   ================================================================
>     2 syscalls:sys_enter_mprotect <= [enter one of the entries]
>     2 syscalls:sys_exit_mprotect
>     13 syscalls:sys_enter_brk
>     ...
> 
> In the hists brower, unfold some of the items, now the cursor can reach
> to any rows:
> 
>     Children      Self  Command  Shared Object          Symbol
>   ================================================================
>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>   - lstat64
>        16.67% 0x6469702e64
>        8.33% 0x646970
>        8.33% 0x617461
>        8.33% 0x65
>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>      0x6469702e64 <= [cursor can reach to bottom line, everything is ok]
> 
> Now, zoom back to "Available samples" and enter again:
> 
>     Children      Self  Command  Shared Object          Symbol
>   ================================================================
>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>   - lstat64
>        16.67% 0x6469702e64
>        8.33% 0x646970
>        8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
>        8.33% 0x65
>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>      0x6469702e64
> 
> This patch recalculates hb->nr_callchain_rows to fix the bug.

Two nits below:
 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/ui/browsers/hists.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 1106bb8..bef9ab0 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -42,6 +42,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb);
>  
>  static struct rb_node *hists__filter_entries(struct rb_node *nd,
>  					     float min_pcnt);
> +static int __hist_browser__get_folding(struct hist_browser *browser);

Why not move the whole function body to...
>  
>  static bool hist_browser__has_filter(struct hist_browser *hb)
>  {

Just before the only caller, i.e. here and...

> @@ -57,6 +58,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
>  	else
>  		nr_entries = hb->hists->nr_entries;
>  
> +	hb->nr_callchain_rows = __hist_browser__get_folding(hb);
>  	return nr_entries + hb->nr_callchain_rows;
>  }
>  
> @@ -353,6 +355,24 @@ static void hist_entry__set_folding(struct hist_entry *he, bool unfold)
>  		he->nr_rows = 0;
>  }
>  
> +static int
> +__hist_browser__get_folding(struct hist_browser *browser)

Why use the __ prefix since there is no hist_browser__get_folding that
calls it and does something more?

Summary: please rename it to hist_browser__get_folding() and move it to
just before its only caller.

Also, besides these nits, are you Ok with this now Namhyung?

Thanks a lot for working on this!

- Arnaldo

> +{
> +	struct rb_node *nd;
> +	struct hists *hists = browser->hists;
> +	int unfolded_rows = 0;
> +
> +	for (nd = rb_first(&hists->entries);
> +	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
> +	     nd = rb_next(nd)) {
> +		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
> +
> +		if (he->ms.unfolded)
> +			unfolded_rows += he->nr_rows;
> +	}
> +	return unfolded_rows;
> +}
> +
>  static void
>  __hist_browser__set_folding(struct hist_browser *browser, bool unfold)
>  {
> -- 
> 2.2.0.33.gc18b867

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol
  2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
@ 2015-03-11 14:12           ` Arnaldo Carvalho de Melo
  2015-03-12  8:05           ` Namhyung Kim
  2015-03-14  7:04           ` [tip:perf/core] " tip-bot for He Kuang
  2 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-03-11 14:12 UTC (permalink / raw)
  To: He Kuang
  Cc: namhyung, a.p.zijlstra, paulus, mingo, jolsa, wangnan0, linux-kernel

Em Wed, Mar 11, 2015 at 08:36:03PM +0800, He Kuang escreveu:
> When zoom into thread/dso/symbol, the fold/unfold stat is cleared in
> hists__filter_by_thread/dso/symbol(), but h->nr_rows is not cleared. So
> if we toggle fold stat on the unfold entires, nr_entries got a wrong
> value.
> 
> This bug can be reproduced as follows:
> 
> $ perf record -g -e syscalls:sys_enter_open ls
> $ perf report

Thanks, bug reproduced and seems fixed after the patch, applying,

- Arnaldo
 
>     Children      Self  Command  Shared Object            Symbol
>   ================================================================
>   +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
>   -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
>       _dl_load_shared_library <= [Zoom into thread/dso]
>       _dl_get_ready_to_run
>       _start
>   ...
> 
> In the new thread hists, all entries reset to fold, if we unfold the
> same entry as we previously unfolded, nr_entries got wrong value, and we
> can't move down cursor to bottom row.
> 
>                                                          Thread: ls
>     Children      Self  Command  Shared Object            Symbol
>   ================================================================
>   +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
>   -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
>       _dl_load_shared_library
>       _dl_get_ready_to_run <= [cursor may stop here, can't move down]
>       _start
>   ...
> 
> This patch clear h->nr_rows to fix this bug.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/hist.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 70b48a6..24aff7d 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
>  	/* force fold unfiltered entry for simplicity */
>  	h->ms.unfolded = false;
>  	h->row_offset = 0;
> +	h->nr_rows = 0;
>  
>  	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
>  
> -- 
> 2.2.0.33.gc18b867

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCHv3] perf hists browser: Fix UI bug after fold/unfold
  2015-03-11 13:48         ` [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold Arnaldo Carvalho de Melo
@ 2015-03-12  7:21           ` He Kuang
  2015-03-12  7:51             ` Namhyung Kim
  2015-03-14  7:05             ` [tip:perf/core] " tip-bot for He Kuang
  0 siblings, 2 replies; 14+ messages in thread
From: He Kuang @ 2015-03-12  7:21 UTC (permalink / raw)
  To: acme, namhyung; +Cc: a.p.zijlstra, paulus, mingo, jolsa, wangnan0, linux-kernel

In perf hists browser, the fold/unfold stat of each hist entry is
recorded but hb->nr_callchain_rows loses its value after zoom out and
zoom in back. This causes a wrong row cursor range that restrict user to
move down anymore.

This bug can be reproduced as follows:

  $ perf record -g -e syscalls:* ls
  $ perf report

    Available samples
  ================================================================
    2 syscalls:sys_enter_mprotect <= [enter one of the entries]
    2 syscalls:sys_exit_mprotect
    13 syscalls:sys_enter_brk
    ...

In the hists brower, unfold some of the items, now the cursor can reach
to any rows:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64 <= [cursor can reach to bottom line, everything is ok]

Now, zoom back to "Available samples" and enter again:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64

This patch recalculates hb->nr_callchain_rows to fix the bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1106bb8..0856209 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -48,6 +48,24 @@ static bool hist_browser__has_filter(struct hist_browser *hb)
 	return hists__has_filter(hb->hists) || hb->min_pcnt;
 }
 
+static int hist_browser__get_folding(struct hist_browser *browser)
+{
+	struct rb_node *nd;
+	struct hists *hists = browser->hists;
+	int unfolded_rows = 0;
+
+	for (nd = rb_first(&hists->entries);
+	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
+	     nd = rb_next(nd)) {
+		struct hist_entry *he =
+			rb_entry(nd, struct hist_entry, rb_node);
+
+		if (he->ms.unfolded)
+			unfolded_rows += he->nr_rows;
+	}
+	return unfolded_rows;
+}
+
 static u32 hist_browser__nr_entries(struct hist_browser *hb)
 {
 	u32 nr_entries;
@@ -57,6 +75,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
 	else
 		nr_entries = hb->hists->nr_entries;
 
+	hb->nr_callchain_rows = hist_browser__get_folding(hb);
 	return nr_entries + hb->nr_callchain_rows;
 }
 
-- 
2.2.0.33.gc18b867


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCHv3] perf hists browser: Fix UI bug after fold/unfold
  2015-03-12  7:21           ` [PATCHv3] " He Kuang
@ 2015-03-12  7:51             ` Namhyung Kim
  2015-03-12 16:19               ` Arnaldo Carvalho de Melo
  2015-03-14  7:05             ` [tip:perf/core] " tip-bot for He Kuang
  1 sibling, 1 reply; 14+ messages in thread
From: Namhyung Kim @ 2015-03-12  7:51 UTC (permalink / raw)
  To: He Kuang; +Cc: acme, a.p.zijlstra, paulus, mingo, jolsa, wangnan0, linux-kernel

On Thu, Mar 12, 2015 at 03:21:49PM +0800, He Kuang wrote:
> In perf hists browser, the fold/unfold stat of each hist entry is
> recorded but hb->nr_callchain_rows loses its value after zoom out and
> zoom in back. This causes a wrong row cursor range that restrict user to
> move down anymore.
> 
> This bug can be reproduced as follows:
> 
>   $ perf record -g -e syscalls:* ls
>   $ perf report
> 
>     Available samples
>   ================================================================
>     2 syscalls:sys_enter_mprotect <= [enter one of the entries]
>     2 syscalls:sys_exit_mprotect
>     13 syscalls:sys_enter_brk
>     ...
> 
> In the hists brower, unfold some of the items, now the cursor can reach
> to any rows:
> 
>     Children      Self  Command  Shared Object          Symbol
>   ================================================================
>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>   - lstat64
>        16.67% 0x6469702e64
>        8.33% 0x646970
>        8.33% 0x617461
>        8.33% 0x65
>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>      0x6469702e64 <= [cursor can reach to bottom line, everything is ok]
> 
> Now, zoom back to "Available samples" and enter again:
> 
>     Children      Self  Command  Shared Object          Symbol
>   ================================================================
>   -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
>   - lstat64
>        16.67% 0x6469702e64
>        8.33% 0x646970
>        8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
>        8.33% 0x65
>   -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
>      0x6469702e64
> 
> This patch recalculates hb->nr_callchain_rows to fix the bug.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 1106bb8..0856209 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -48,6 +48,24 @@ static bool hist_browser__has_filter(struct hist_browser *hb)
>  	return hists__has_filter(hb->hists) || hb->min_pcnt;
>  }
>  
> +static int hist_browser__get_folding(struct hist_browser *browser)
> +{
> +	struct rb_node *nd;
> +	struct hists *hists = browser->hists;
> +	int unfolded_rows = 0;
> +
> +	for (nd = rb_first(&hists->entries);
> +	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
> +	     nd = rb_next(nd)) {
> +		struct hist_entry *he =
> +			rb_entry(nd, struct hist_entry, rb_node);
> +
> +		if (he->ms.unfolded)
> +			unfolded_rows += he->nr_rows;
> +	}
> +	return unfolded_rows;
> +}
> +
>  static u32 hist_browser__nr_entries(struct hist_browser *hb)
>  {
>  	u32 nr_entries;
> @@ -57,6 +75,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
>  	else
>  		nr_entries = hb->hists->nr_entries;
>  
> +	hb->nr_callchain_rows = hist_browser__get_folding(hb);
>  	return nr_entries + hb->nr_callchain_rows;
>  }
>  
> -- 
> 2.2.0.33.gc18b867
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol
  2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
  2015-03-11 14:12           ` Arnaldo Carvalho de Melo
@ 2015-03-12  8:05           ` Namhyung Kim
  2015-03-14  7:04           ` [tip:perf/core] " tip-bot for He Kuang
  2 siblings, 0 replies; 14+ messages in thread
From: Namhyung Kim @ 2015-03-12  8:05 UTC (permalink / raw)
  To: He Kuang; +Cc: a.p.zijlstra, paulus, mingo, acme, jolsa, wangnan0, linux-kernel

On Wed, Mar 11, 2015 at 08:36:03PM +0800, He Kuang wrote:
> When zoom into thread/dso/symbol, the fold/unfold stat is cleared in
> hists__filter_by_thread/dso/symbol(), but h->nr_rows is not cleared. So
> if we toggle fold stat on the unfold entires, nr_entries got a wrong
> value.
> 
> This bug can be reproduced as follows:
> 
> $ perf record -g -e syscalls:sys_enter_open ls
> $ perf report
> 
>     Children      Self  Command  Shared Object            Symbol
>   ================================================================
>   +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
>   -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
>       _dl_load_shared_library <= [Zoom into thread/dso]
>       _dl_get_ready_to_run
>       _start
>   ...
> 
> In the new thread hists, all entries reset to fold, if we unfold the
> same entry as we previously unfolded, nr_entries got wrong value, and we
> can't move down cursor to bottom row.
> 
>                                                          Thread: ls
>     Children      Self  Command  Shared Object            Symbol
>   ================================================================
>   +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
>   -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
>       _dl_load_shared_library
>       _dl_get_ready_to_run <= [cursor may stop here, can't move down]
>       _start
>   ...
> 
> This patch clear h->nr_rows to fix this bug.
> 
> Signed-off-by: He Kuang <hekuang@huawei.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/util/hist.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 70b48a6..24aff7d 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1169,6 +1169,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
>  	/* force fold unfiltered entry for simplicity */
>  	h->ms.unfolded = false;
>  	h->row_offset = 0;
> +	h->nr_rows = 0;
>  
>  	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
>  
> -- 
> 2.2.0.33.gc18b867
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv3] perf hists browser: Fix UI bug after fold/unfold
  2015-03-12  7:51             ` Namhyung Kim
@ 2015-03-12 16:19               ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-03-12 16:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: He Kuang, a.p.zijlstra, paulus, mingo, jolsa, wangnan0, linux-kernel

Em Thu, Mar 12, 2015 at 04:51:10PM +0900, Namhyung Kim escreveu:
> On Thu, Mar 12, 2015 at 03:21:49PM +0800, He Kuang wrote:
> > This patch recalculates hb->nr_callchain_rows to fix the bug.
> > 
> > Signed-off-by: He Kuang <hekuang@huawei.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tip:perf/core] perf hists browser: Fix UI bug after zoom into thread/dso/symbol
  2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
  2015-03-11 14:12           ` Arnaldo Carvalho de Melo
  2015-03-12  8:05           ` Namhyung Kim
@ 2015-03-14  7:04           ` tip-bot for He Kuang
  2 siblings, 0 replies; 14+ messages in thread
From: tip-bot for He Kuang @ 2015-03-14  7:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: namhyung, hpa, linux-kernel, tglx, a.p.zijlstra, hekuang, paulus,
	acme, mingo, jolsa, wangnan0

Commit-ID:  a8cd1f4393032cd87e98803346865cdbceb15ad3
Gitweb:     http://git.kernel.org/tip/a8cd1f4393032cd87e98803346865cdbceb15ad3
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Wed, 11 Mar 2015 20:36:03 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 12:39:59 -0300

perf hists browser: Fix UI bug after zoom into thread/dso/symbol

When zoom into thread/dso/symbol, the fold/unfold stat is cleared in
hists__filter_by_thread/dso/symbol(), but h->nr_rows is not cleared. So
if we toggle fold stat on the unfold entires, nr_entries got a wrong
value.

This bug can be reproduced as follows:

$ perf record -g -e syscalls:sys_enter_open ls
$ perf report

    Children      Self  Command  Shared Object            Symbol
  ================================================================
  +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
  -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
      _dl_load_shared_library <= [Zoom into thread/dso]
      _dl_get_ready_to_run
      _start
  ...

In the new thread hists, all entries reset to fold, if we unfold the
same entry as we previously unfolded, nr_entries got wrong value, and we
can't move down cursor to bottom row.

                                                         Thread: ls
    Children      Self  Command  Shared Object            Symbol
  ================================================================
  +   50.00%     0.00%  ls       ld64.so  [.]  _dl_get_ready_to_run
  -   50.00%     0.00%  ls       ld64.so  [.]  _dl_load_shared_library
      _dl_load_shared_library
      _dl_get_ready_to_run <= [cursor may stop here, can't move down]
      _start
  ...

This patch clear h->nr_rows to fix this bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1426077363-855-2-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 95f5ab7..d9a6d35 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1171,6 +1171,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h
 	/* force fold unfiltered entry for simplicity */
 	h->ms.unfolded = false;
 	h->row_offset = 0;
+	h->nr_rows = 0;
 
 	hists->stats.nr_non_filtered_samples += h->stat.nr_events;
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [tip:perf/core] perf hists browser: Fix UI bug after fold/unfold
  2015-03-12  7:21           ` [PATCHv3] " He Kuang
  2015-03-12  7:51             ` Namhyung Kim
@ 2015-03-14  7:05             ` tip-bot for He Kuang
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot for He Kuang @ 2015-03-14  7:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, mingo, tglx, hpa, paulus, linux-kernel, acme, namhyung,
	wangnan0, a.p.zijlstra, hekuang

Commit-ID:  4fabf3d19cec11d9faa567f8cf0290298c5a93ea
Gitweb:     http://git.kernel.org/tip/4fabf3d19cec11d9faa567f8cf0290298c5a93ea
Author:     He Kuang <hekuang@huawei.com>
AuthorDate: Thu, 12 Mar 2015 15:21:49 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 12 Mar 2015 13:18:38 -0300

perf hists browser: Fix UI bug after fold/unfold

In perf hists browser, the fold/unfold stat of each hist entry is
recorded but hb->nr_callchain_rows loses its value after zoom out and
zoom in back. This causes a wrong row cursor range that restrict user to
move down anymore.

This bug can be reproduced as follows:

  $ perf record -g -e syscalls:* ls
  $ perf report

    Available samples
  ================================================================
    2 syscalls:sys_enter_mprotect <= [enter one of the entries]
    2 syscalls:sys_exit_mprotect
    13 syscalls:sys_enter_brk
    ...

In the hists brower, unfold some of the items, now the cursor can reach
to any rows:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64 <= [cursor can reach to bottom line, everything is ok]

Now, zoom back to "Available samples" and enter again:

    Children      Self  Command  Shared Object          Symbol
  ================================================================
  -  100.00%   100.00%  ls       libuClibc-0.9.33.2.so  [.] lstat64
  - lstat64
       16.67% 0x6469702e64
       8.33% 0x646970
       8.33% 0x617461 <= [cursor may stop here, can't move down anymore]
       8.33% 0x65
  -   16.67%     0.00%  ls       [unknown]              [.]0x6469702e64
     0x6469702e64

This patch recalculates hb->nr_callchain_rows to fix the bug.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1426144909-18951-1-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index ad312d9..49eddeb 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -48,6 +48,24 @@ static bool hist_browser__has_filter(struct hist_browser *hb)
 	return hists__has_filter(hb->hists) || hb->min_pcnt;
 }
 
+static int hist_browser__get_folding(struct hist_browser *browser)
+{
+	struct rb_node *nd;
+	struct hists *hists = browser->hists;
+	int unfolded_rows = 0;
+
+	for (nd = rb_first(&hists->entries);
+	     (nd = hists__filter_entries(nd, browser->min_pcnt)) != NULL;
+	     nd = rb_next(nd)) {
+		struct hist_entry *he =
+			rb_entry(nd, struct hist_entry, rb_node);
+
+		if (he->ms.unfolded)
+			unfolded_rows += he->nr_rows;
+	}
+	return unfolded_rows;
+}
+
 static u32 hist_browser__nr_entries(struct hist_browser *hb)
 {
 	u32 nr_entries;
@@ -57,6 +75,7 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
 	else
 		nr_entries = hb->hists->nr_entries;
 
+	hb->nr_callchain_rows = hist_browser__get_folding(hb);
 	return nr_entries + hb->nr_callchain_rows;
 }
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-03-14  7:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-06 12:51 [PATCH] perf hists browser: Fix UI bug after fold/unfold He Kuang
2015-03-10  6:28 ` Namhyung Kim
2015-03-10  7:26   ` He Kuang
2015-03-11  0:26     ` Namhyung Kim
2015-03-11 12:36       ` [PATCHv2 1/2] " He Kuang
2015-03-11 12:36         ` [PATCHv2 2/2] perf hists browser: Fix UI bug after zoom into thread/dso/symbol He Kuang
2015-03-11 14:12           ` Arnaldo Carvalho de Melo
2015-03-12  8:05           ` Namhyung Kim
2015-03-14  7:04           ` [tip:perf/core] " tip-bot for He Kuang
2015-03-11 13:48         ` [PATCHv2 1/2] perf hists browser: Fix UI bug after fold/unfold Arnaldo Carvalho de Melo
2015-03-12  7:21           ` [PATCHv3] " He Kuang
2015-03-12  7:51             ` Namhyung Kim
2015-03-12 16:19               ` Arnaldo Carvalho de Melo
2015-03-14  7:05             ` [tip:perf/core] " tip-bot for He Kuang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).