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

Em Tue, Aug 10, 2021 at 12:31:55PM +0200, Riccardo Mancini escreveu:

Hi Arnaldo,

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

quoted

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

Yeah, patches are welcome, but at least lets not add new ones :-)
 

quoted

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.

I'd have to do some extra research to remember why that is also not
optimal, IIRC the '= { .a = 0, };' is the optimal one.
 

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?

Nah, as this is the HEAD right now in my local branch, I'll apply it
myself, thanks!

- Arnaldo
 

-- 

- Arnaldo