linux-kernel - Re: [PATCH linux-next] perf report: remove warning on missing raw data for s390

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 22 Nov 2023 18:34:14 -0800
From:   Namhyung Kim <namhyung@...nel.org>
To:     Thomas Richter <tmricht@...ux.ibm.com>
Cc:     linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        acme@...nel.org, svens@...ux.ibm.com, gor@...ux.ibm.com,
        sumanthk@...ux.ibm.com, hca@...ux.ibm.com
Subject: Re: [PATCH linux-next] perf report: remove warning on missing raw
 data for s390

Hello,

On Wed, Nov 22, 2023 at 1:27 AM Thomas Richter <tmricht@...ux.ibm.com> wrote:
>
> Command
>  # ./perf report -i /tmp/111 -D > /dev/null
> emits an error message when a sample for event CRYPTO_ALL in the
> perf.data file does not contain any raw data. This is ok.
> Do not trigger this warning when the sample in the perf.data files
> does not contain any raw data at all.
> Check for availability of raw data for all events and return if
> none is available.
>
> Output before:
>  # ./perf report -i /tmp/111 -D > /dev/null
>  Invalid CRYPTO_ALL raw data encountered
>  Invalid CRYPTO_ALL raw data encountered
>  Invalid CRYPTO_ALL raw data encountered
>  #
>
> Output after:
>  # ./perf report -i /tmp/111 -D > /dev/null
>  #
>
> Fixes: b539deafbadb ("perf report: Add s390 raw data interpretation for PAI counters")
> Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com>
> Acked-by: Sumanth Korikkar <sumanthk@...ux.ibm.com>

Acked-by: Namhyung Kim <namhyung@...nel.org>

> ---
>  tools/perf/util/s390-sample-raw.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
> index 29a744eeb71e..53383e97ec9d 100644
> --- a/tools/perf/util/s390-sample-raw.c
> +++ b/tools/perf/util/s390-sample-raw.c
> @@ -51,8 +51,6 @@ static bool s390_cpumcfdg_testctr(struct perf_sample *sample)
>         struct cf_trailer_entry *te;
>         struct cf_ctrset_entry *cep, ce;
>
> -       if (!len)
> -               return false;
>         while (offset < len) {
>                 cep = (struct cf_ctrset_entry *)(buf + offset);
>                 ce.def = be16_to_cpu(cep->def);
> @@ -234,10 +232,9 @@ struct pai_data {          /* Event number and value */
>   */
>  static bool s390_pai_all_test(struct perf_sample *sample)
>  {
> -       unsigned char *buf = sample->raw_data;
>         size_t len = sample->raw_size;
>
> -       if (len < 0xa || !buf)
> +       if (len < 0xa)
>                 return false;
>         return true;
>  }
> @@ -299,6 +296,10 @@ void evlist__s390_sample_raw(struct evlist *evlist, union perf_event *event,
>         if (!evsel)
>                 return;
>
> +       /* Check for raw data in sample */
> +       if (!sample->raw_size || !sample->raw_data)
> +               return;

I was thinking it should check if evsel->attr.sample_type has
PERF_SAMPLE_RAW.  But evsel__parse_sample() sets the
sample->raw_data and ->raw_size only if it has the flag.
So I think it's fine.

Thanks,
Namhyung


> +
>         /* Display raw data on screen */
>         if (evsel->core.attr.config == PERF_EVENT_CPUM_CF_DIAG) {
>                 if (!evsel->pmu)
> --
> 2.41.0
>
>