Re: [RFC PATCH v8 2/4] arm64: Reorganize the unwinder code for better consistency and maintenance

From: Madhavan T. Venkataraman
Date: Thu Aug 26 2021 - 19:19:21 EST




On 8/26/21 10:46 AM, Mark Brown wrote:
> On Thu, Aug 12, 2021 at 02:06:01PM -0500, madvenka@xxxxxxxxxxxxxxxxxxx wrote:
>
>> Renaming of unwinder functions
>> ==============================
>
>> Rename unwinder functions to unwind_*() similar to other architectures
>> for naming consistency. More on this below.
>
> This feels like it could probably do with splitting up a bit for
> reviewability, several of these headers you've got in the commit
> logs look like they could be separate commits. Splitting things
> up does help with reviewability, having only one change to keep
> in mind at once is a lot less cognative load.
>
>> Replace walk_stackframe() with unwind()
>> =======================================
>>
>> walk_stackframe() contains the unwinder loop that walks the stack
>> frames. Currently, start_backtrace() and walk_stackframe() are called
>> separately. They should be combined in the same function. Also, the
>> loop in walk_stackframe() should be simplified and should look like
>> the unwind loops in other architectures such as X86 and S390.
>
> This definitely seems like a separate change.
>

OK. I will take a look at splitting the patch.

I am also requesting a review of the sym_code special section approach.
I know that you have already approved it. I wanted one more vote. Then,
I can remove the "RFC" word from the title and then it will be just a
code review of the patch series.

Mark Rutland,

Do you also approve the idea of placing unreliable functions (from an unwind
perspective) in a special section and using that in the unwinder for
reliable stack trace?

Thanks.

Madhavan