linux-kernel - Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aEIIRmb6SNOroSJJ@x1>
Date: Thu, 5 Jun 2025 18:12:38 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org,
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
Subject: Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type
 display

On Thu, Jun 05, 2025 at 05:46:22PM -0300, Arnaldo Carvalho de Melo wrote:
> On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> > Support data type display with a key press so that users can toggle the
> > output dynamically on TUI.
> 
> 'd', 'T', control+d, control+t are all available, why 'y'? :-)
> 
> Apart from these minor nits,

There is one issue I noticed now that Ingo mentioned earlier, if you
press 'y' to get Data type display we better add some visual cue that
this is toggled, like was done for source code view in:

commit c6043d35c0f3eb5bcbeb6309e10c4ae33d203841
Author: Arnaldo Carvalho de Melo <acme@...hat.com>
Date:   Tue Apr 8 19:03:43 2025 -0300

    perf ui browser annotate: Show in the title the source code view toggle
    
    Ingo reported that having a visual cue if the source code view is
    enabled will help in noticing a bug when no source is presented.
    
    Change the title scnprintf routine for the annotation browser to do
    that.
    
    Suggested-by: Ingo Molnar <mingo@...nel.org>

Also I did it with a simple perf.data file, no data stuff in it,
probably it is best to show that if what is required for it to work is
present in the perf.data file.

Or to tell what is needed for that hotkey/feature to work, which I think
is even better and more informative.

- Arnaldo
 
> Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> 
> - Arnaldo
>  
> > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > ---
> >  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 686fa54e545e275c..cd1d452035a265d3 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		"b             Toggle percent base [period/hits]\n"
> >  		"B             Branch counter abbr list (Optional)\n"
> >  		"?             Search string backwards\n"
> > -		"f             Toggle showing offsets to full address\n");
> > +		"f             Toggle showing offsets to full address\n"
> > +		"y             Toggle data type display\n");
> >  			continue;
> >  		case 'r':
> >  			script_browse(NULL, NULL);
> > @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		case 'f':
> >  			annotation__toggle_full_addr(notes, ms);
> >  			continue;
> > +		case 'y':
> > +			annotate_opts.code_with_type ^= 1;
> > +			if (browser->dbg == NULL)
> > +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> > +			continue;
> >  		case K_LEFT:
> >  		case '<':
> >  		case '>':
> > @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >  
> >  	ret = annotate_browser__run(&browser, evsel, hbt);
> >  
> > -	if (annotate_opts.code_with_type)
> > -		debuginfo__delete(browser.dbg);
> > +	debuginfo__delete(browser.dbg);
> >  	if(not_annotated)
> >  		annotated_source__purge(notes->src);
> >  
> > -- 
> > 2.49.0
> >