All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Aditya Gupta <adityag@linux.ibm.com>,
	jolsa@kernel.org, irogers@google.com,
	linux-perf-users@vger.kernel.org, maddy@linux.ibm.com,
	atrajeev@linux.vnet.ibm.com, kjain@linux.ibm.com,
	disgoel@linux.vnet.ibm.com
Subject: Re: [PATCH v11 1/4] perf check: introduce check subcommand
Date: Fri, 28 Jun 2024 16:28:06 -0300	[thread overview]
Message-ID: <Zn8OxjRgFlPduRKh@x1> (raw)
In-Reply-To: <Zn8AvuvyjXFGa90c@google.com>

On Fri, Jun 28, 2024 at 11:28:14AM -0700, Namhyung Kim wrote:
> On Fri, Jun 28, 2024 at 11:24:53AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> > And while looking at it:
> > 
> >              get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
> > 
> > This looks wrong, no? Or at least confusing, have to check the source
> > code...
> 
> We have this in Makefile.config
> 
> ifndef NO_AUXTRACE
>   ifeq ($(SRCARCH),x86)
>     ifeq ($(feature-get_cpuid), 0)
>       $(warning Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc)
>       NO_AUXTRACE := 1
>     endif
>   endif

The complete sequence is:

ifndef NO_AUXTRACE
  ifeq ($(SRCARCH),x86)
    ifeq ($(feature-get_cpuid), 0)
      $(warning Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc)
      NO_AUXTRACE := 1
    endif
  endif
  ifndef NO_AUXTRACE
    $(call detected,CONFIG_AUXTRACE)
    CFLAGS += -DHAVE_AUXTRACE_SUPPORT
    ifeq ($(feature-reallocarray), 0)
      CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
    endif
  endif
endif

The most descriptive would be to HAVE_GET_CPUID_SUPPORT and have it used
in the source code.

That or have:

diff --git a/tools/perf/builtin-check.c b/tools/perf/builtin-check.c
index 44ffde6f8dbe51f3..ae4a686ff4f265be 100644
--- a/tools/perf/builtin-check.c
+++ b/tools/perf/builtin-check.c
@@ -33,7 +33,7 @@ struct feature_status supported_features[] = {
 	FEATURE_STATUS("dwarf", HAVE_DWARF_SUPPORT),
 	FEATURE_STATUS("dwarf_getlocations", HAVE_DWARF_GETLOCATIONS_SUPPORT),
 	FEATURE_STATUS("dwarf-unwind-support", HAVE_DWARF_UNWIND_SUPPORT),
-	FEATURE_STATUS("get_cpuid", HAVE_AUXTRACE_SUPPORT),
+	FEATURE_STATUS("auxtrace", HAVE_AUXTRACE_SUPPORT),
 	FEATURE_STATUS("libaudit", HAVE_LIBAUDIT_SUPPORT),
 	FEATURE_STATUS("libbfd", HAVE_LIBBFD_SUPPORT),
 	FEATURE_STATUS("libcapstone", HAVE_LIBCAPSTONE_SUPPORT),

That:

        FEATURE_STATUS("dwarf-unwind-support", HAVE_DWARF_UNWIND_SUPPORT),

Should also really be:

        FEATURE_STATUS("dwarf-unwind", HAVE_DWARF_UNWIND_SUPPORT),

For consistency, the get_cpuid/auxtrace also for consistency, I think.

- Arnaldo

  reply	other threads:[~2024-06-28 19:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-27 10:06 [PATCH v11 0/4] Introduce perf check subcommand Aditya Gupta
2024-06-27 10:06 ` [PATCH v11 1/4] perf check: introduce " Aditya Gupta
2024-06-28 14:12   ` Arnaldo Carvalho de Melo
2024-06-28 14:15     ` Arnaldo Carvalho de Melo
2024-06-28 14:20       ` Arnaldo Carvalho de Melo
2024-06-28 14:24         ` Arnaldo Carvalho de Melo
2024-06-28 18:28           ` Namhyung Kim
2024-06-28 19:28             ` Arnaldo Carvalho de Melo [this message]
2024-06-30 14:08               ` Aditya Gupta
2024-07-02 21:28                 ` Arnaldo Carvalho de Melo
2024-06-30 11:01           ` Aditya Gupta
2024-06-30 10:43         ` Aditya Gupta
2024-06-30 10:41       ` Aditya Gupta
2024-06-30 10:30     ` Aditya Gupta
2024-06-27 10:06 ` [PATCH v11 2/4] perf version: update --build-options to use 'supported_features' array Aditya Gupta
2024-06-27 10:06 ` [PATCH v11 3/4] perf tests task_analyzer: use perf check for libtraceevent support Aditya Gupta
2024-06-27 10:06 ` [PATCH v11 4/4] tools/perf/tests: Update probe_vfs_getname.sh script to use perf check feature Aditya Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zn8OxjRgFlPduRKh@x1 \
    --to=acme@kernel.org \
    --cc=adityag@linux.ibm.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=disgoel@linux.vnet.ibm.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kjain@linux.ibm.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=maddy@linux.ibm.com \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.