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

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

[Thomas Richter]

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@linux.ibm.com>
Acked-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
---
 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;
+
 	/* Display raw data on screen */
 	if (evsel->core.attr.config == PERF_EVENT_CPUM_CF_DIAG) {
 		if (!evsel->pmu)
-- 
2.41.0

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

[Namhyung Kim]

Hello,

On Wed, Nov 22, 2023 at 1:27 AM Thomas Richter <tmricht@linux.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@linux.ibm.com>
> Acked-by: Sumanth Korikkar <sumanthk@linux.ibm.com>

Acked-by: Namhyung Kim <namhyung@kernel.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
>
>

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

[Arnaldo Carvalho de Melo]

Em Wed, Nov 22, 2023 at 06:34:14PM -0800, Namhyung Kim escreveu:
> Hello,
> 
> On Wed, Nov 22, 2023 at 1:27 AM Thomas Richter <tmricht@linux.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@linux.ibm.com>
> > Acked-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools-next.

- Arnaldo