Re: [PATCH] perf bench: add benchmark for evlist open/close operations

Hi Arnaldo,

On Mon, 2021-08-09 at 17:28 -0300, Arnaldo Carvalho de Melo wrote:

Em Mon, Aug 09, 2021 at 05:23:31PM -0300, Arnaldo Carvalho de Melo escreveu:

quoted

Em Mon, Aug 09, 2021 at 10:11:02PM +0200, Riccardo Mancini escreveu:

quoted

+static struct evlist *bench__create_evlist(char *evstr)
+{
+       struct evlist *evlist;
+       struct parse_events_error err;
+       int ret;

quoted

quoted

+       evlist = evlist__new();
+       if (!evlist) {
+               pr_err("Not enough memory to create evlist\n");
+               return NULL;
+       }

quoted

quoted

+       bzero(&err, sizeof(err));

quoted

man bzero

       The bzero() function is deprecated (marked as LEGACY in POSIX.1-2001);
use memset(3) in new programs.  POSIX.1-2008 removes the specification of
bzero().  The bzero() function first appeared in 4.3BSD.

Oops, I didn't know, but I saw it is being used in some parts in perf, maybe we
should get rid of them:
$ rg -c bzero
builtin-lock.c:1
arch/powerpc/util/kvm-stat.c:1
builtin-stat.c:1
builtin-trace.c:2
bench/evlist-open-close.c:1
bench/numa.c:5
tests/parse-events.c:1
tests/backward-ring-buffer.c:1
tests/bpf.c:2
util/metricgroup.c:1
util/parse-events.c:1

quoted

I'm replacing it with a memset().

This one is also equivalent:

tools/perf/tests/pmu-events.c:  struct parse_events_error error = { .idx = 0, };

https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

That text is a bit roundabout, as it says that the members that are not
explicitely initialized will be initialized as variables with static
storage duration, i.e. zeroed.

Would it be the same doing the shorter {0}. It would be a general solution for
these init-to-zero cases.

Unrelated to this small issue, I noticed I forgot to check the return of
bench__create_evlist. Would you like me to send a v2 fixing both issues or are
you able to apply this other small change yourself?

diff --git a/tools/perf/bench/evlist-open-close.c b/tools/perf/bench/evlist-open-close.c
index 40bce06f5ca7bef3..f0b9c330f34f2984 100644
--- a/tools/perf/bench/evlist-open-close.c
+++ b/tools/perf/bench/evlist-open-close.c

@@ -168,7 +168,11 @@ static int bench_evlist_open_close__run(char *evstr)
 
        for (i = 0; i < iterations; i++) {
                pr_debug("Started iteration %d\n", i);
+
                evlist = bench__create_evlist(evstr);
+               if (!evlist)
+                       return -ENOMEM;
+
                gettimeofday(&start, NULL);
                err = bench__do_evlist_open_close(evlist);
                if (err) {

Thanks,
Riccardo

- Arnaldo