Commit 900e14a8f5a49e987790b93c7906989b22075f1b
1 parent
7588badafc
Exists in
master
and in
6 other branches
perf hists browser: Recalculate browser pointers after resort/decay
In browsers that access dynamic underlying data structures, like in the hists browser and its hist_entry rb_tree, we need to revalidate any reference to the underlying data structure, because they can have gone away, decayed. This fixes a problem where after a while the top entries get behind the top of the screen, i.e. the top_idx stays at 0, which means it is at the first entry in the rb_tree when in fact it wasn't because the browser->top didn't got revalidated after the timer ran and the underlying data structure got updated. Cc: David Ahern <dsahern@gmail.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/n/tip-mhje66qssdko24q67a2lhlho@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Showing 3 changed files with 26 additions and 7 deletions Side-by-side Diff
tools/perf/util/ui/browser.c
| ... | ... | @@ -230,6 +230,29 @@ |
| 230 | 230 | return 0; |
| 231 | 231 | } |
| 232 | 232 | |
| 233 | +/* | |
| 234 | + * Here we're updating nr_entries _after_ we started browsing, i.e. we have to | |
| 235 | + * forget about any reference to any entry in the underlying data structure, | |
| 236 | + * that is why we do a SEEK_SET. Think about 'perf top' in the hists browser | |
| 237 | + * after an output_resort and hist decay. | |
| 238 | + */ | |
| 239 | +void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries) | |
| 240 | +{ | |
| 241 | + off_t offset = nr_entries - browser->nr_entries; | |
| 242 | + | |
| 243 | + browser->nr_entries = nr_entries; | |
| 244 | + | |
| 245 | + if (offset < 0) { | |
| 246 | + if (browser->top_idx < (u64)-offset) | |
| 247 | + offset = -browser->top_idx; | |
| 248 | + | |
| 249 | + browser->index += offset; | |
| 250 | + browser->top_idx += offset; | |
| 251 | + } | |
| 252 | + | |
| 253 | + browser->seek(browser, browser->top_idx, SEEK_SET); | |
| 254 | +} | |
| 255 | + | |
| 233 | 256 | int ui_browser__run(struct ui_browser *self) |
| 234 | 257 | { |
| 235 | 258 | struct newtExitStruct es; |
tools/perf/util/ui/browser.h
| ... | ... | @@ -41,6 +41,7 @@ |
| 41 | 41 | void ui_browser__hide(struct ui_browser *self); |
| 42 | 42 | int ui_browser__refresh(struct ui_browser *self); |
| 43 | 43 | int ui_browser__run(struct ui_browser *self); |
| 44 | +void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries); | |
| 44 | 45 | |
| 45 | 46 | void ui_browser__rb_tree_seek(struct ui_browser *self, off_t offset, int whence); |
| 46 | 47 | unsigned int ui_browser__rb_tree_refresh(struct ui_browser *self); |
tools/perf/util/ui/browsers/hists.c
| ... | ... | @@ -332,13 +332,7 @@ |
| 332 | 332 | case -1: |
| 333 | 333 | /* FIXME we need to check if it was es.reason == NEWT_EXIT_TIMER */ |
| 334 | 334 | timer(arg); |
| 335 | - /* | |
| 336 | - * The timer may have changed the number of entries. | |
| 337 | - * XXX: Find better way to keep this in synch, probably | |
| 338 | - * removing this timer function altogether and just sync | |
| 339 | - * using the hists->lock... | |
| 340 | - */ | |
| 341 | - self->b.nr_entries = self->hists->nr_entries; | |
| 335 | + ui_browser__update_nr_entries(&self->b, self->hists->nr_entries); | |
| 342 | 336 | hists__browser_title(self->hists, title, sizeof(title), |
| 343 | 337 | ev_name, self->dso_filter, |
| 344 | 338 | self->thread_filter); |
| ... | ... | @@ -985,6 +979,7 @@ |
| 985 | 979 | |
| 986 | 980 | hist_entry__tui_annotate(he, evsel->idx, nr_events, |
| 987 | 981 | timer, arg, delay_secs); |
| 982 | + ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries); | |
| 988 | 983 | } else if (choice == browse_map) |
| 989 | 984 | map__browse(browser->selection->map); |
| 990 | 985 | else if (choice == zoom_dso) { |