On 2/2/22 18:26, Madhavan T. Venkataraman wrote: > > > On 2/2/22 12:44, Mark Brown wrote: >> On Mon, Jan 17, 2022 at 08:56:01AM -0600, madvenka@xxxxxxxxxxxxxxxxxxx wrote: >> >>> +/* >>> + * TODO: document requirements here. >>> + */ >>> +static inline void unwind_init_from_regs(struct unwind_state *state, >>> + struct pt_regs *regs) >> >>> +/* >>> + * TODO: document requirements here. >>> + * >>> + * Note: this is always inlined, and we expect our caller to be a noinline >>> + * function, such that this starts from our caller's caller. >>> + */ >>> +static __always_inline void unwind_init_from_current(struct unwind_state *state) >> >>> +/* >>> + * TODO: document requirements here. >>> + * >>> + * The caller guarantees that the task is not running. >>> + */ >>> +static inline void unwind_init_from_task(struct unwind_state *state, >>> + struct task_struct *task) >> >> Other than the obvious gap this looks good to me. For _current() I >> don't think we've got any particular requirements other than what's >> documented. For the others I think the main thing is that trying to >> walk the stack of a task that is actively executing is going to be a bad >> idea so we should say that the task shouldn't be running, but in general >> given that one of the main use cases is printing diagnostics on error >> we shouldn't have too many *requirements* for calling these. > > OK. For now, I will remove the TODO comment from individual functions. > I will add only a common general comment above all 3 helpers that > additional requirements may be documented as seen fit. And, I will > add that the task must not be running in other-directed cases. > If what I have suggested above for comments is good enough, can I get a Reviewed-by for this? I will fix the comments on the next send. > Madhavan