On 10/27/21 12:53 PM, Mark Rutland wrote:
> On Thu, Oct 14, 2021 at 09:58:44PM -0500, madvenka@xxxxxxxxxxxxxxxxxxx wrote:
>> From: "Madhavan T. Venkataraman" <madvenka@xxxxxxxxxxxxxxxxxxx>
>>
>> Rename unwinder functions for consistency and better naming.
>>
>> - Rename start_backtrace() to unwind_start().
>> - Rename unwind_frame() to unwind_next().
>> - Rename walk_stackframe() to unwind().
>
> This looks good to me.
>
Thanks.
> Could we split this from the krpbes/tracing changes? I think this stands
> on it's own, and (as below) the kprobes/tracing changes need some more
> explanation, and would make sense as a separate patch.
>
OK. I will split the patches.
>> Prevent the following unwinder functions from being traced:
>>
>> - unwind_start()
>> - unwind_next()
>>
>> unwind() is already prevented from being traced.
>
> This could do with an explanation in the commis message as to why we
> need to do this. If this is fixing a latent issue, it should be in a
> preparatory patch that we can backport.
>
> I dug into this a bit, and from taking a look, we prohibited ftrace in commit:
>
> 0c32706dac1b0a72 ("arm64: stacktrace: avoid tracing arch_stack_walk()")
>
> ... which is just one special case of graph return stack unbalancing,
> and should be addressed by using HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, so
> with the patch making us use HAVE_FUNCTION_GRAPH_RET_ADDR_PTR, that's
> no longer necessary.
>
> So we no longer seem to have a specific reason to prohibit ftrace
> here.
>
OK, I will think about this and add a comment.
>> Prevent the following unwinder functions from being kprobed:
>>
>> - unwind_start()
>>
>> unwind_next() and unwind() are already prevented from being kprobed.
>
> Likewise, I think this needs some explanation. From diggin, we
> prohibited kprobes in commit:
>
> ee07b93e7721ccd5 ("arm64: unwind: Prohibit probing on return_address()")
>
> ... and the commit message says we need to do this because this is
> (transitively) called by trace_hardirqs_off(), which is kprobes
> blacklisted, but doesn't explain the actual problem this results in.
>
OK. I will think about this and add a comment.
> AFAICT x86 directly uses __builtin_return_address() here, but that won't
> recover rewritten addresses, which seems like a bug (or at least a
> limitation) on x86, assuming I've read that correctly.
>
OK.
Thanks,
Madhavan
