From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
To: mingo@redhat.com, acme@kernel.org, mhiramat@kernel.org
Cc: brendan.d.gregg@gmail.com, peterz@infradead.org,
        alexander.shishkin@linux.intel.com, wangnan0@huawei.com,
        jolsa@kernel.org, ak@linux.intel.com, treeze.taeung@gmail.com,
        mathieu.poirier@linaro.org, hekuang@huawei.com,
        sukadev@linux.vnet.ibm.com, ananth@in.ibm.com,
        naveen.n.rao@linux.vnet.ibm.com, adrian.hunter@intel.com,
        linux-kernel@vger.kernel.org, hemant@linux.vnet.ibm.com,
        Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Subject: [PATCH v4 5/7] perf/sdt: Warn when number of events recorded are not equal to cached events
Date: Mon,  6 Mar 2017 18:43:02 +0530
In-Reply-To: <20170306131304.1625-1-ravi.bangoria@linux.vnet.ibm.com>
References: <20170306131304.1625-1-ravi.bangoria@linux.vnet.ibm.com>
Message-Id: <20170306131304.1625-6-ravi.bangoria@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4543
Lines: 126

If number of events found from probe-cache is not equal to number of
existing events(fetched from uprobe_events), and somehow we decides
to record only existing events, we warn user about the same. For ex,

  $ sudo ./perf probe sdt_libpthread:mutex_release
    Added new events:
      sdt_libpthread:mutex_release (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_1 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_2 (on %mutex_release in /usr/lib64/libpthread-2.24.so)
      sdt_libpthread:mutex_release_3 (on %mutex_release in /usr/lib64/libpthread-2.24.so)

  $ sudo ./perf record -a -e sdt_libpthread:*
    Warning: Recording on 4 occurrences of sdt_libpthread:*
    Warning: Found 35 events from probe-cache with name 'sdt_libpthread:*'.
             Since 4 probe points already exists, recording only them.
    Hint: Please use 'perf probe -d sdt_libpthread:*' to allow record on all events.

  $ sudo ./perf evlist
    sdt_libpthread:mutex_release_3
    sdt_libpthread:mutex_release_2
    sdt_libpthread:mutex_release_1
    sdt_libpthread:mutex_release

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 59 ++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 358ca98..90444e5 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -951,24 +951,6 @@ void free_sdt_list(struct list_head *sdt_evlist)
 	}
 }
 
-static int get_sdt_events_from_cache(struct perf_probe_event *pev)
-{
-	int ret = 0;
-
-	pev->ntevs = find_cached_events_all(pev, &pev->tevs);
-
-	if (pev->ntevs < 0) {
-		pr_err("Error: Cache lookup failed (code: %d)\n", pev->ntevs);
-		ret = pev->ntevs;
-	} else if (!pev->ntevs) {
-		pr_err("Error: %s:%s not found in the cache\n",
-			pev->group, pev->event);
-		ret = -EINVAL;
-	}
-
-	return ret;
-}
-
 static int add_event_to_sdt_evlist(struct probe_trace_event *tev,
 				   struct list_head *sdt_evlist,
 				   bool exst)
@@ -1076,6 +1058,17 @@ static void shift_sdt_events(struct perf_probe_event *pev, int i)
 	pev->ntevs--;
 }
 
+static void sdt_warn_abt_exist_events(struct perf_probe_event *pev, int ctr)
+{
+	pr_warning("Warning: Found %d events from probe-cache with name '%s:%s'.\n"
+		"\t Since %d probe point%c already exists, recording only %s.\n"
+		"Hint: Please use 'perf probe -d %s:%s' to allow record on all events.\n\n",
+		pev->ntevs, pev->group, pev->event, ctr,
+		ctr > 1 ? 's' : '\0',
+		ctr > 1 ? "them" : "it",
+		pev->group, pev->event);
+}
+
 static int sdt_merge_events(struct perf_probe_event *pev,
 			    struct probe_trace_event *exst_tevs,
 			    int exst_ntevs,
@@ -1155,6 +1148,15 @@ int add_sdt_event(char *event, struct list_head *sdt_evlist)
 	probe_conf.max_probes = MAX_PROBES;
 	probe_conf.force_add = 1;
 
+	/*
+	 * This call is intentionally placed before fetching events
+	 * from uprobe_events file. If number of events found from probe-
+	 * cache is not equal to number of existing events, and somehow
+	 * we decides to record only existing events, we warn user about
+	 * the same (sdt_warn_abt_exist_events()).
+	 */
+	pev->ntevs = find_cached_events_all(pev, &pev->tevs);
+
 	/* Fetch all sdt events from uprobe_events */
 	exst_ntevs = get_exist_sdt_events(&exst_tevs);
 	if (exst_ntevs < 0) {
@@ -1166,14 +1168,29 @@ int add_sdt_event(char *event, struct list_head *sdt_evlist)
 	ret = sdt_event_probepoint_exists(pev, exst_tevs,
 					 exst_ntevs, sdt_evlist);
 	if (ret) {
+		if (ret > 0 && pev->ntevs > 0 && ret != pev->ntevs)
+			sdt_warn_abt_exist_events(pev, ret);
 		ret = ret > 0 ? 0 : ret;
 		goto free_pev;
 	}
 
-	/* Fetch all matching events from cache. */
-	ret = get_sdt_events_from_cache(pev);
-	if (ret < 0)
+	/*
+	 * Check if find_cached_events_all() failed.
+	 * We deliberately check failure of this function after checking
+	 * entries in uprobe_events. Because, even if this function fails,
+	 * we may find matching entry from uprobe_events and in that case
+	 * we should continue recording that event.
+	 */
+	if (pev->ntevs < 0) {
+		pr_err("Error: Cache lookup failed (code: %d)\n", pev->ntevs);
+		ret = pev->ntevs;
 		goto free_pev;
+	} else if (!pev->ntevs) {
+		pr_err("Error: %s:%s not found in the cache\n",
+			pev->group, pev->event);
+		ret = -EINVAL;
+		goto free_pev;
+	}
 
 	/*
 	 * Merge events found from uprobe_events with events found
-- 
2.9.3