Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <3b0abd4a-6450-c282-65ff-ec0c7df67b03@amd.com> <20230621062359.201-1-ravi.bangoria@amd.com>
In-Reply-To: <20230621062359.201-1-ravi.bangoria@amd.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Wed, 21 Jun 2023 15:33:53 -0700
Message-ID: <CAM9d7chbpiSJ6eSu9_Gs7aLZZb58gtHsvH9tsE-U4eM1eyDvKQ@mail.gmail.com>
Subject: Re: [PATCH] perf evsel amd: Fix IBS error message
To:     Ravi Bangoria <ravi.bangoria@amd.com>
Cc:     acme@kernel.org, peterz@infradead.org, irogers@google.com,
        jolsa@kernel.org, adrian.hunter@intel.com,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        sandipan.das@amd.com, ananth.narayan@amd.com,
        santosh.shukla@amd.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Ravi,

On Tue, Jun 20, 2023 at 11:24 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>
> AMD IBS can do per-process profiling[1] and is no longer restricted to
> per-cpu or systemwide only. Remove stale error message.
>
> Before:
>   $ sudo ./perf record -e "{ibs_op//,ibs_op//}" -C 0
>   Error:
>   AMD IBS may only be available in system-wide/per-cpu mode.
>   Try using -a, or -C and workload affinity

It was strange that the -C option was given already.

>
> After:
>   $ sudo ./perf record -e "{ibs_op//,ibs_op//}" -C 0
>   Error:
>   The sys_perf_event_open() syscall returned with 22 (Invalid
>   argument) for event (ibs_op//).
>   /bin/dmesg | grep -i perf may provide additional information.

It can run newer perf tools on an old kernel but the old error
message seems to be invalid anyway.  So I'm ok with removing it.

>
> [1] https://git.kernel.org/torvalds/c/30093056f7b2
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
> ---
>  tools/perf/util/evsel.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 356c07f03be6..65b0b70830f0 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3092,14 +3092,10 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
>                         return scnprintf(msg, size,
>         "Invalid event (%s) in per-thread mode, enable system wide with '-a'.",
>                                         evsel__name(evsel));
> -               if (is_amd(arch, cpuid)) {
> -                       if (is_amd_ibs(evsel)) {
> -                               if (evsel->core.attr.exclude_kernel)
> -                                       return scnprintf(msg, size,
> +               if (is_amd(arch, cpuid) && is_amd_ibs(evsel)) {
> +                       if (evsel->core.attr.exclude_kernel) {
> +                               return scnprintf(msg, size,
>         "AMD IBS can't exclude kernel events.  Try running at a higher privilege level.");

I'm not sure if it's enough.  The IBS PMUs have CAP_NO_EXCLUDE then
it can't exclude user events too, right?

Thanks,
Namhyung


> -                               if (!evsel->core.system_wide)
> -                                       return scnprintf(msg, size,
> -       "AMD IBS may only be available in system-wide/per-cpu mode.  Try using -a, or -C and workload affinity");
>                         }
>                 }
>
> --
> 2.41.0
>