Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1343344

Re: [PATCH net-next 1/3] perf: generalize perf_callchain

Show all headers | View raw

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.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH net-next 1/3] perf: generalize perf_callchain Peter Zijlstra <peterz@infradead.org> - 2016-02-25 17:50 +0100
  Re: [PATCH net-next 1/3] perf: generalize perf_callchain Alexei Starovoitov <ast@fb.com> - 2016-02-25 18:30 +0100

csiph-web