Re: Recent Power changes and stack_trace_save_tsk_reliable?

[Joe Lawrence <joe.lawrence@redhat.com>]

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.

--
Joe

> 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
> 
> 
> cheers
>