Re: [RFC PATCH v1 1/1] arm64: Unwinder enhancements for reliable stack trace
From: Madhavan T. Venkataraman
Date: Wed Feb 24 2021 - 14:28:03 EST
On 2/24/21 6:33 AM, Mark Brown wrote:
>
On Tue, Feb 23, 2021 at 01:20:49PM -0600, Madhavan T. Venkataraman wrote:
>
> On 2/23/21 1:02 PM, Mark Brown wrote:
>
>> On Tue, Feb 23, 2021 at 12:12:43PM -0600, madvenka@xxxxxxxxxxxxxxxxxxx wrote:
>
>
>>> Reliable stack trace function
>
>>> =============================
>
>>>
>
>>> Implement arch_stack_walk_reliable(). This function walks the stack like
>
>>> the existing stack trace functions with a couple of additional checks:
>
>
>> Again, this should be at least one separate patch. How does this ensure
>
>> that we don't have any issues with any of the various probe mechanisms?
>
>> If there's no need to explicitly check anything that should be called
>
>> out in the changelog.
>
>
> I am trying to do this in an incremental fashion. I have to study the probe
>
> mechanisms a little bit more before I can come up with a solution. But
>
> if you want to see that addressed in this patch set, I could do that.
>
> It will take a little bit of time. That is all.
>
>
Handling of the probes stuff seems like it's critical to reliable stack
>
walk so we shouldn't claim to have support for reliable stack walk
>
without it. If it was a working implementation we could improve that'd
>
be one thing but this would be buggy which is a different thing.
>
OK. I will address the probe stuff in my resend.
>
>>> + (void) on_accessible_stack(task, stackframe, &info);
>
>
>> Shouldn't we return NULL if we are not on an accessible stack?
>
>
> The prev_fp has already been checked by the unwinder in the previous
>
> frame. That is why I don't check the return value. If that is acceptable,
>
> I will add a comment.
>
>
TBH if you're adding the comment it seems like you may as well add the
>
check, it's not like it's expensive and it means there's no possibility
>
that some future change could result in this assumption being broken.
>
OK. I will add the check.
Thanks.
Madhavan