Re: [PATCH 3/5] perf util: Introduce architecture specific errno/name mapping

[Hendrik Brueckner <brueckner@linux.vnet.ibm.com>]

On Thu, Jan 18, 2018 at 07:49:53AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 18, 2018 at 11:07:33AM +0100, Jiri Olsa escreveu:
> > On Tue, Jan 16, 2018 at 10:46:29AM +0100, Hendrik Brueckner wrote:
> > 
> > SNIP
> > 
> > > index 12dec6e..90ce14f 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -249,7 +249,7 @@ INC_FLAGS += -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi
> > >  INC_FLAGS += -I$(srctree)/tools/arch/$(SRCARCH)/include/
> > >  INC_FLAGS += -I$(srctree)/tools/arch/$(SRCARCH)/
> > >  
> > > -# $(obj-perf)      for generated common-cmds.h
> > > +# $(obj-perf)      for generated common-cmds.h and errno-names.c
> > >  # $(obj-perf)/util for generated bison/flex headers
> > >  ifneq ($(OUTPUT),)
> > >  INC_FLAGS += -I$(obj-perf)/util
> > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > index 9fdefd7..332b4b4 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -518,6 +518,9 @@ $(OUTPUT)common-cmds.h: util/generate-cmdlist.sh command-list.txt
> > >  $(OUTPUT)common-cmds.h: $(wildcard Documentation/perf-*.txt)
> > >  	$(QUIET_GEN). util/generate-cmdlist.sh > $@+ && mv $@+ $@
> > >  
> > > +$(OUTPUT)util/errno-names.c: util/generate-errno-names.sh
> > > +	$(QUIET_GEN). util/generate-errno-names.sh "$(CC)" "$(srctree)/tools" > $@+ && mv $@+ $@
> > 
> > so this generates errno-names.c with following code:
> > 
> > const char *arch_errno_to_name(const char *arch, int err)
> > {
> >         if (!strcmp(arch, "alpha"))
> >                 return errno_to_name__alpha(err);
> >         if (!strcmp(arch, "arm"))
> >                 return errno_to_name__arm(err);
> >         if (!strcmp(arch, "arm64"))
> >                 return errno_to_name__arm64(err);
> > 
> > 
> > is there a reason why we want all archs compiled in?
> > I'd think we just need the arch perf is built in
> 
> think about:
> 
> On an arm64 system:
> 
> 	perf trace record -a sleep 1
> 
> scp that to an x86_64 system and do:
> 
> 	perf trace -i perf.data
> 
> But doing that strcmp per call seems excessive, perhaps we could have
> something like:
> 
> 	trace->arch = arch_syscalls__init(arch_name);
> 
> 	char *s = arch_syscalls__strerrno(trace->arch, err);
> 
> Have to look at the code to find the right naming according to what we
> already have.

Another idea to get rid of the strcmp's is to introduce an arch enum
and let normalize_arch() in util/env.c return an enum number and handle
the arch stuff then on the enum.  Where necessary, the arch string can
be used.  But for "internal" purposes, an enum would be better.

Thanks and kind regards,
  Hendrik