From: Mark Brown <firstname.lastname@example.org> To: "Madhavan T. Venkataraman" <email@example.com> Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [RFC PATCH v1 1/1] arm64: Unwinder enhancements for reliable stack trace Date: Wed, 24 Feb 2021 12:33:36 +0000 [thread overview] Message-ID: <20210224123336.GA4504@sirena.org.uk> (raw) In-Reply-To: <firstname.lastname@example.org> [-- Attachment #1: Type: text/plain, Size: 1746 bytes --] 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, email@example.com 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. > >> + (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. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-02-24 12:35 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <bc4761a47ad08ab7fdd555fc8094beb8fc758d33> 2021-02-23 18:12 ` [RFC PATCH v1 0/1] " madvenka 2021-02-23 18:12 ` [RFC PATCH v1 1/1] " madvenka 2021-02-23 19:02 ` Mark Brown 2021-02-23 19:20 ` Madhavan T. Venkataraman 2021-02-24 12:33 ` Mark Brown [this message] 2021-02-24 19:26 ` Madhavan T. Venkataraman 2021-02-24 12:17 ` Mark Rutland 2021-02-24 19:34 ` Madhavan T. Venkataraman 2021-02-25 11:58 ` Mark Rutland 2021-03-01 16:58 ` Madhavan T. Venkataraman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210224123336.GA4504@sirena.org.uk \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [RFC PATCH v1 1/1] arm64: Unwinder enhancements for reliable stack trace' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).