Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 15 Dec 2020 12:34:48 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Tiezhu Yang <yangtiezhu@loongson.cn>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Xuefeng Li <lixuefeng@loongson.cn>
Subject: Re: [PATCH v2] perf callchain: Return directly when use
 '--call-graph dwarf' under !HAVE_DWARF_SUPPORT
Message-ID: <20201215153448.GA258566@kernel.org>
References: <1607996131-9340-1-git-send-email-yangtiezhu@loongson.cn>
 <CAM9d7chMkKBschy=abDqyOBg8_jxXBXhSN30k2m+MhPca_g2ig@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAM9d7chMkKBschy=abDqyOBg8_jxXBXhSN30k2m+MhPca_g2ig@mail.gmail.com>
Precedence: bulk

Em Tue, Dec 15, 2020 at 10:49:59PM +0900, Namhyung Kim escreveu:
> Hello,
> 
> On Tue, Dec 15, 2020 at 10:35 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > DWARF register mappings have not been defined for some architectures,
> > at least for mips, so we can print an error message and then return
> > directly when use '--call-graph dwarf'.
> >
> > E.g. without this patch:
> >
> > [root@linux perf]# ./perf record --call-graph dwarf cd
> > Error:
> > The sys_perf_event_open() syscall returned with 89 (Function not implemented) for event (cycles).
> > /bin/dmesg | grep -i perf may provide additional information.
> >
> > With this patch:
> >
> > [root@linux perf]# ./perf record --call-graph dwarf cd
> > DWARF is not supported for architecture mips64
> >
> >  Usage: perf record [<options>] [<command>]
> >     or: perf record [<options>] -- <command> [<options>]
> >
> >         --call-graph <record_mode[,record_size]>
> >                           setup and enables call-graph (stack chain/backtrace):
> >
> >                                 record_mode:    call graph recording mode (fp|dwarf|lbr)
> >                                 record_size:    if record_mode is 'dwarf', max size of stack recording (<bytes>)
> >                                                 default: 8192 (bytes)
> >
> >                                 Default: fp
> >
> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> > ---
> >
> > v2: Use HAVE_DWARF_SUPPORT to check
> 
> I'm not sure whether this is because of lack of dwarf library or kernel support.
> Based on the error message, I guess it's from the kernel.  Then I think this
> patch won't be sufficient..

tools/perf/Makefile.config

  ifndef NO_DWARF
    ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
      msg := $(warning DWARF register mappings have not been defined for architecture $(SRCARCH), DWARF support disabled);
      NO_DWARF := 1
    else
      CFLAGS += -DHAVE_DWARF_SUPPORT $(LIBDW_CFLAGS)
      LDFLAGS += $(LIBDW_LDFLAGS)
      EXTLIBS += ${DWARFLIBS}
      $(call detected,CONFIG_DWARF)
    endif # PERF_HAVE_DWARF_REGS
  endif # NO_DWARF

[acme@five perf]$ find tools/perf -type f | xargs grep PERF_HAVE_DWARF_REGS
tools/perf/arch/xtensa/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/x86/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/arm64/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/arm/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/s390/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/powerpc/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/sh/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/riscv/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/sparc/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/arch/csky/Makefile:PERF_HAVE_DWARF_REGS := 1
tools/perf/Makefile.config:    ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
tools/perf/Makefile.config:    endif # PERF_HAVE_DWARF_REGS
[acme@five perf]$

Ouch:

[acme@five perf]$ cat tools/perf/arch/xtensa/Makefile
# SPDX-License-Identifier: GPL-2.0-only
ifndef NO_DWARF
PERF_HAVE_DWARF_REGS := 1
endif
[acme@five perf]$

So you have a point, only if NO_DWARF is not defined, then
PERF_HAVE_DWARF_REGS is can be used to define HAVE_DWARF_SUPPORT, too
clumsy.

So probably my hunch that this should be done at
evsel__open_strerror() and use perf_missing_features.dwarf_regs is
what we should go...

I'm removing this patch from my current tree, please take a look at:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/core&id=f55c66234c1967f6e56e56c3e084f80b417c124b

For how I did this for another feature that the kernel may or not
support, perf_event_open() fails, it notices that in
perf_missing_features and then later evsel__open_strerror() returns a
sensible warning, reacting to something the running kernel returned.

- Arnaldo