Re: [PATCH 3/3] perf tool: give user better message if precise isnot supported

From: David Ahern
Date: Tue Sep 11 2012 - 09:22:32 EST


On 9/11/12 3:20 AM, Robert Richter wrote:
On 10.09.12 10:40:16, David Ahern wrote:
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -294,6 +294,11 @@ try_again:
perf_evsel__name(pos));
rc = -err;
goto out;
+ } else if ((err == ENOTSUP) && (attr->precise_ip)) {

It is EOPNOTSUPP, did you test this?

I do not post patches without testing them. This particular patch was verified in a Virtual Machine (no PEBS) and using :pG modifier.

'egrep -r ENOTSUP tools/perf' shows hits in 3 other files, so I am not the only one using the shortcut. I'll change it in the follow up with better commit messages to make it consistent with patch 2.


+ ui__error("\'precise\' request not supported. "
+ "Try removing 'p' modifier\n");

I would better print "... request may not be supported.", since you
don't know for sure if this is the real cause.

Sure I'll add the 'may not be'.


+ rc = -err;
+ goto out;
}

printf("\n");
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0513aaa..0d3653b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -975,6 +975,10 @@ try_again:
ui__error("Too many events are opened.\n"
"Try again after reducing the number of events\n");
goto out_err;
+ } else if ((err == ENOTSUP) && (attr->precise_ip)) {
+ ui__error("\'precise\' request not supported. "
+ "Try removing 'p' modifier\n");

Same here.

+ goto out_err;

To avoid adding more duplicate code, maybe we should start to unify
the code by implementing this in a shared helper function.

Doing that requires additional modifications to not break perl and python scripts. Adding it in both commands here is consistent with all other open counter failures. Consolidation of those loops into a common base is known to do item.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/