All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.