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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Thu, Jan 18, 2018 at 12:33:20PM +0100, Hendrik Brueckner escreveu:
> 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.

Right, we can go on improving things, but I think that after you get the
generated directory sorted out I'll apply your patches in the current
form, and then we can go on improving things from there, ok?

- Arnaldo