Re: Recent Power changes and stack_trace_save_tsk_reliable?

[Petr Mladek <pmladek@suse.com>]

On Wed 2023-08-30 17:47:35, Joe Lawrence wrote:
> On 8/30/23 02:37, Michael Ellerman wrote:
> > Michael Ellerman <mpe@ellerman.id.au> writes:
> >> Joe Lawrence <joe.lawrence@redhat.com> writes:
> >>> Hi ppc-dev list,
> >>>
> >>> We noticed that our kpatch integration tests started failing on ppc64le
> >>> when targeting the upstream v6.4 kernel, and then confirmed that the
> >>> in-tree livepatching kselftests similarly fail, too.  From the kselftest
> >>> results, it appears that livepatch transitions are no longer completing.
> >>
> >> Hi Joe,
> >>
> >> Thanks for the report.
> >>
> >> I thought I was running the livepatch tests, but looks like somewhere
> >> along the line my kernel .config lost CONFIG_TEST_LIVEPATCH=m, so I have
> >> been running the test but it just skips. :/
> >>
> 
> That config option is easy to drop if you use `make localmodconfig` to
> try and expedite the builds :D  Been there, done that too many times.
> 
> >> I can reproduce the failure, and will see if I can bisect it more
> >> successfully.
> > 
> > It's caused by:
> > 
> >   eed7c420aac7 ("powerpc: copy_thread differentiate kthreads and user mode threads")
> > 
> > Which is obvious in hindsight :)
> > 
> > The diff below fixes it for me, can you test that on your setup?
> > 
> 
> Thanks for the fast triage of this one.  The proposed fix works well on
> our setup.  I have yet to try the kpatch integration tests with this,
> but I can verify that all of the kernel livepatching kselftests now
> happily run.

Have this been somehow handled, please? I do not see the proposed
change in linux-next as of now.

> > A proper fix will need to be a bit bigger because the comments in there
> > are all slightly wrong now since the above commit.
> > 
> > Possibly we can also rework that code more substantially now that
> > copy_thread() is more careful about setting things up, but that would be
> > a follow-up.
> > 
> > diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
> > index 5de8597eaab8..d0b3509f13ee 100644
> > --- a/arch/powerpc/kernel/stacktrace.c
> > +++ b/arch/powerpc/kernel/stacktrace.c
> > @@ -73,7 +73,7 @@ int __no_sanitize_address arch_stack_walk_reliable(stack_trace_consume_fn consum
> >  	bool firstframe;
> >  
> >  	stack_end = stack_page + THREAD_SIZE;
> > -	if (!is_idle_task(task)) {
> > +	if (!(task->flags & PF_KTHREAD)) {
> >  		/*
> >  		 * For user tasks, this is the SP value loaded on
> >  		 * kernel entry, see "PACAKSAVE(r13)" in _switch() and

If I read the change in the commit eed7c420aac7fde ("powerpc: copy_thread
differentiate kthreads and user mode threads") correctly then the
above fix is correct.

It is probably just enough to update the comment about that
STACK_FRAME_MIN_SIZE is used by all kthreads.

Best Regards,
Petr