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
