On 2/25/16 8:47 AM, Peter Zijlstra wrote:
On Wed, Feb 17, 2016 at 07:58:57PM -0800, Alexei Starovoitov wrote:
+static inline int perf_callchain_store(struct perf_callchain_entry *entry, u64
ip)
{
+ if (entry->nr < PERF_MAX_STACK_DEPTH) {
entry->ip[entry->nr++] = ip;
+ return 0;
+ } else {
+ return -1; /* no more room, stop walking the stack */
+ }
}
Why 0 and -1 ?
because that's the interface you had for callbacks in
'struct stacktrace_ops' including a comment there:
/* On negative return stop dumping */
What's wrong with something like:
static inline bool perf_callchain_store(struct perf_callchain_entry *entry, u64
ip)
{
if (entry->nr < PERF_MAX_STACK_DEPTH) {
entry->ip[entry->nr++] = ip;
return true;
}
return false;
}
I would prefer something like this as well, but it would look
inconsistent with what is already there. To make it bool
one would need to change struct stacktrace_ops for all archs
and touch a lot of files all over the tree.
Way more than 51 insertions(+), 29 deletions(-) as this patch did
for no real gain.
It's better to be consistent with existing code.
