All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Song Liu <songliubraving@fb.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"alexander.shishkin@linux.intel.com" 
	<alexander.shishkin@linux.intel.com>,
	"namhyung@kernel.org" <namhyung@kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"jolsa@redhat.com" <jolsa@redhat.com>,
	Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH v6 3/4] perf-stat: enable counting events for BPF programs
Date: Tue, 29 Dec 2020 16:23:47 -0300	[thread overview]
Message-ID: <20201229192347.GM521329@kernel.org> (raw)
In-Reply-To: <20201229191848.GL521329@kernel.org>

Em Tue, Dec 29, 2020 at 04:18:48PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 29, 2020 at 07:11:12PM +0000, Song Liu escreveu:
> > > On Dec 29, 2020, at 10:48 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > I'll check this one to get a patch that at least moves the needle here,
> > > i.e. probably we can leave supporting bpf counters in the python binding
> > > for a later step.

> > Thanks Arnaldo!

> > Currently, I have:
> > 1. Fixed issues highlighted by Namhyung;
> > 2. Merged 3/4 and 4/4;
> > 3. NOT found segfault;
> > 4. NOT fixed python import perf. 

> > I don't have good ideas with 3 and 4... Shall I send current code as v7?

> For 4, please fold the patch below into the relevant patch, we don't
> need bpf_counter.h included in util/evsel.h, you even added a forward
> declaration for that 'struct bpf_counter_ops'.
 
> And in general we should refrain from adding extra includes to header
> files, .h-ell is not good.
> 
> Then we provide a stub for that bpf_counter__destroy() so that
> util/evsel.o when linked into the perf python biding find it there,
> links ok.

Ok, one more stub is needed, I wasn't building all the time with 

  $ make BUILD_BPF_SKEL=1

Ditch the previous patch please, use the one below instead:

- Arnaldo

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 40e3946cd7518113..8226b1fefa8cf2a3 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -10,7 +10,6 @@
 #include <internal/evsel.h>
 #include <perf/evsel.h>
 #include "symbol_conf.h"
-#include "bpf_counter.h"
 #include <internal/cpumap.h>
 
 struct bpf_object;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index cc5ade85a33fc999..278abecb5bdfc0d2 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -79,6 +79,27 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 	return 0;
 }
 
+/*
+ * XXX: All these evsel destructors need some better mechanism, like a linked
+ * list of destructors registered when the relevant code indeed is used instead
+ * of having more and more calls in perf_evsel__delete(). -- acme
+ *
+ * For now, add some more:
+ *
+ * Not to drag the BPF bandwagon...
+ */
+void bpf_counter__destroy(struct evsel *evsel);
+int bpf_counter__install_pe(struct evsel *evsel, int cpu, int fd);
+
+void bpf_counter__destroy(struct evsel *evsel __maybe_unused)
+{
+}
+
+int bpf_counter__install_pe(struct evsel *evsel __maybe_unused, int cpu __maybe_unused, int fd __maybe_unused)
+{
+	return 0;
+}
+
 /*
  * Support debug printing even though util/debug.c is not linked.  That means
  * implementing 'verbose' and 'eprintf'.

  reply	other threads:[~2020-12-29 19:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-28 17:40 [PATCH v6 0/4] Introduce perf-stat -b for BPF programs Song Liu
2020-12-28 17:40 ` [PATCH v6 1/4] bpftool: add Makefile target bootstrap Song Liu
2020-12-28 17:40 ` [PATCH v6 2/4] perf: support build BPF skeletons with perf Song Liu
2020-12-29  7:01   ` Namhyung Kim
2020-12-29 11:48     ` Arnaldo Carvalho de Melo
2020-12-29 17:14       ` Song Liu
2020-12-29 18:16         ` Arnaldo Carvalho de Melo
2020-12-28 17:40 ` [PATCH v6 3/4] perf-stat: enable counting events for BPF programs Song Liu
2020-12-28 20:11   ` Arnaldo Carvalho de Melo
2020-12-28 23:43     ` Song Liu
2020-12-29  5:53       ` Song Liu
2020-12-29 15:15       ` Arnaldo Carvalho de Melo
2020-12-29 18:42         ` Song Liu
2020-12-29 18:48           ` Arnaldo Carvalho de Melo
2020-12-29 19:11             ` Song Liu
2020-12-29 19:18               ` Arnaldo Carvalho de Melo
2020-12-29 19:23                 ` Arnaldo Carvalho de Melo [this message]
2020-12-29 19:32                   ` Arnaldo Carvalho de Melo
2020-12-29 21:40                     ` Song Liu
2020-12-29  7:22   ` Namhyung Kim
2020-12-29 17:46     ` Song Liu
2020-12-29 17:59       ` Song Liu
2020-12-28 17:40 ` [PATCH v6 4/4] perf-stat: add documentation for -b option Song Liu
2020-12-29  7:24   ` Namhyung Kim
2020-12-29 16:59     ` Song Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201229192347.GM521329@kernel.org \
    --to=acme@kernel.org \
    --cc=Kernel-team@fb.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=songliubraving@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.