Re: [PATCH 28/28] perf tools: Add udis86 disassembler feature check

[Ingo Molnar]

* Jiri Olsa <jolsa@xxxxxxxxxx> wrote:

> On Wed, Dec 04, 2013 at 03:50:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 03, 2013 at 02:09:42PM +0100, Jiri Olsa escreveu:
> > > Adding udis86 disassembler feature check which support
> > > is needed for kvm:kvm_emulate_insn tracepoint.
> > >  
> > > +$(call feature_check,udis86)
> > > +ifeq ($(feature-udis86), 1)
> > > +  LIBTRACEEVENT_CFLAGS += -DHAVE_UDIS86
> > > +  EXTLIBS += -ludis86
> > > +else
> > > +  msg := $(warning No udis86 support.);
> > > +endif
> > 
> > That is really an incomplete message, what package should I install?
> > Perhaps we should add this there then:
> > 
> > http://bit.ly/1hyrN52
> 
> nice :-)
> 
> > 
> > Wow, that was easy, but yeah, could be made easier 8-) ;-P
> 
> so something like:
> No udis86 found. Please install udis86-devel.
> 
> or:
> No udis86 found, disabling kvm tracepoints instruction disassembly. Please install udis86-devel.

1)

So, the first problem I can see is in the output:

Auto-detecting system features:
...                     backtrace: [ on  ]
...                         dwarf: [ on  ]
...                fortify-source: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                  gtk2-infobar: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...             libelf-getphdrnum: [ on  ]
...                   libelf-mmap: [ on  ]
...                       libnuma: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...             libpython-version: [ on  ]
...                      libslang: [ on  ]
...                     libunwind: [ on  ]
...                       on-exit: [ on  ]
...            stackprotector-all: [ on  ]
...                       timerfd: [ on  ]

config/Makefile:421: No udis86 support.

such a 'no XYZ support' message should only occur if a feature test 
failed. _If_ we decide that 'udis86' support is required for a full 
perf build then there should be a new line saying something like:

...                        udis86: [ OFF ]

2)

The other problem I see is that I think we should start adding the 
install-suggestions to the feature-check lines themselves.

On .deb based systems it should say something like:

...                       on-exit: [ on  ]
...            stackprotector-all: [ on  ]
...                       timerfd: [ on  ]
...                        udis86: [ OFF ]    # apt-get install udis86-dev

On RPM based systems it should say something like:

...                       on-exit: [ on  ]
...            stackprotector-all: [ on  ]
...                       timerfd: [ on  ]
...                        udis86: [ OFF ]    # yum install udis86-devel

Or so. I think we can maintain package suggestion strings for these 
two main distro variants.

The package install strings should probably be maintained together 
with the list of feature names in some table/list format. Such lookup 
tables can be implemented either in Make via:

  http://www.gnu.org/savannah-checkouts/gnu/make/manual/html_node/Computed-Names.html

Or via a shell command in the makefile, using Bash associative arrays 
or such.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/