[PATCH] stacktrace: do not trace user stack for user_worker tasks

[PATCH] stacktrace: do not trace user stack for user_worker tasks

[Jiazi Li]

Tasks with PF_USER_WORKER flag also only run in kernel space,
so do not trace user stack for these tasks.

Signed-off-by: Jiazi Li <jqqlijiazi@gmail.com>
Signed-off-by: peixuan.qiu <peixuan.qiu@transsion.com>
---
 kernel/stacktrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index afb3c116da91..82fbccdd1a24 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -228,8 +228,8 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
 		.size	= size,
 	};
 
-	/* Trace user stack if not a kernel thread */
-	if (current->flags & PF_KTHREAD)
+	/* Skip tasks that do not return to userspace */
+	if (current->flags & (PF_KTHREAD | PF_USER_WORKER))
 		return 0;
 
 	arch_stack_walk_user(consume_entry, &c, task_pt_regs(current));
-- 
2.49.0

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Steven Rostedt]

On Mon, 23 Jun 2025 19:59:11 +0800
Jiazi Li <jqqlijiazi@gmail.com> wrote:

> Tasks with PF_USER_WORKER flag also only run in kernel space,
> so do not trace user stack for these tasks.

What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?

Has all the locations that test for PF_KTHREAD been audited to make
sure that PF_USER_WORKER isn't also needed?

I'm working on other code that needs to differentiate between user
tasks and kernel tasks, and having to have multiple flags to test is
becoming quite a burden.

-- Steve


> 
> Signed-off-by: Jiazi Li <jqqlijiazi@gmail.com>
> Signed-off-by: peixuan.qiu <peixuan.qiu@transsion.com>
> ---
>  kernel/stacktrace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> index afb3c116da91..82fbccdd1a24 100644
> --- a/kernel/stacktrace.c
> +++ b/kernel/stacktrace.c
> @@ -228,8 +228,8 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
>  		.size	= size,
>  	};
>  
> -	/* Trace user stack if not a kernel thread */
> -	if (current->flags & PF_KTHREAD)
> +	/* Skip tasks that do not return to userspace */
> +	if (current->flags & (PF_KTHREAD | PF_USER_WORKER))
>  		return 0;
>  
>  	arch_stack_walk_user(consume_entry, &c, task_pt_regs(current));

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Jiazi Li]

On Tue, Jun 24, 2025 at 01:07:44PM -0400, Steven Rostedt wrote:
> On Mon, 23 Jun 2025 19:59:11 +0800
> Jiazi Li <jqqlijiazi@gmail.com> wrote:
> 
> > Tasks with PF_USER_WORKER flag also only run in kernel space,
> > so do not trace user stack for these tasks.
> 
> What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?
> 
I think that apart from never return to user space, PF_USER_WORKER is
basically the same as user space task.
> Has all the locations that test for PF_KTHREAD been audited to make
> sure that PF_USER_WORKER isn't also needed?
> 
No.
> I'm working on other code that needs to differentiate between user
> tasks and kernel tasks, and having to have multiple flags to test is
> becoming quite a burden.
> 
Yes, so only check both PF_KTHREAD and PF_USER_WORKER before access 
user space stack?
> -- Steve
> 
> 
> > 
> > Signed-off-by: Jiazi Li <jqqlijiazi@gmail.com>
> > Signed-off-by: peixuan.qiu <peixuan.qiu@transsion.com>
> > ---
> >  kernel/stacktrace.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> > index afb3c116da91..82fbccdd1a24 100644
> > --- a/kernel/stacktrace.c
> > +++ b/kernel/stacktrace.c
> > @@ -228,8 +228,8 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
> >  		.size	= size,
> >  	};
> >  
> > -	/* Trace user stack if not a kernel thread */
> > -	if (current->flags & PF_KTHREAD)
> > +	/* Skip tasks that do not return to userspace */
> > +	if (current->flags & (PF_KTHREAD | PF_USER_WORKER))
> >  		return 0;
> >  
> >  	arch_stack_walk_user(consume_entry, &c, task_pt_regs(current));
> 

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Jens Axboe]

On 6/24/25 11:07 AM, Steven Rostedt wrote:
> On Mon, 23 Jun 2025 19:59:11 +0800
> Jiazi Li <jqqlijiazi@gmail.com> wrote:
> 
>> Tasks with PF_USER_WORKER flag also only run in kernel space,
>> so do not trace user stack for these tasks.
> 
> What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?

One is a kernel thread (eg no mm, etc), the other is basically a user
thread. None of them exit to userspace, that's basically the only
thing they have in common.

> Has all the locations that test for PF_KTHREAD been audited to make
> sure that PF_USER_WORKER isn't also needed?

I did when adding it, to the best of my knowledge. But there certainly
could still be gaps. Sometimes not easy to see why code checks for
PF_KTHREAD in the first place.

> I'm working on other code that needs to differentiate between user
> tasks and kernel tasks, and having to have multiple flags to test is
> becoming quite a burden.

None of them are user tasks, but PF_USER_WORKER does look like a
user thread and acts like one, except it wasn't created by eg
pthread_create() and it never returns to userspace. When it's done,
it's simply reaped.

-- 
Jens Axboe

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Steven Rostedt]

[
  Adding Peter Zijlstra as he has been telling me to test against
  PF_KTHREAD instead of current->mm to tell if it is a kernel thread.
  But that seems to not be enough!
]

On Wed, 25 Jun 2025 10:23:28 -0600
Jens Axboe <axboe@kernel.dk> wrote:

> On 6/24/25 11:07 AM, Steven Rostedt wrote:
> > On Mon, 23 Jun 2025 19:59:11 +0800
> > Jiazi Li <jqqlijiazi@gmail.com> wrote:
> >   
> >> Tasks with PF_USER_WORKER flag also only run in kernel space,
> >> so do not trace user stack for these tasks.  
> > 
> > What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?  
> 
> One is a kernel thread (eg no mm, etc), the other is basically a user
> thread. None of them exit to userspace, that's basically the only
> thing they have in common.

Was it ever in user space? Because exiting isn't the issue for getting
a user space stack. If it never was in user space than sure, there's no
reason to look at the user space stack.

> 
> > Has all the locations that test for PF_KTHREAD been audited to make
> > sure that PF_USER_WORKER isn't also needed?  
> 
> I did when adding it, to the best of my knowledge. But there certainly
> could still be gaps. Sometimes not easy to see why code checks for
> PF_KTHREAD in the first place.
> 
> > I'm working on other code that needs to differentiate between user
> > tasks and kernel tasks, and having to have multiple flags to test is
> > becoming quite a burden.  
> 
> None of them are user tasks, but PF_USER_WORKER does look like a
> user thread and acts like one, except it wasn't created by eg
> pthread_create() and it never returns to userspace. When it's done,
> it's simply reaped.
> 

I'm assuming that it also never was in user space, which is where we
don't want to do any user space stack trace.

This looks like more rationale for having a kernel_task() user_task()
helper functions:

  https://lore.kernel.org/linux-trace-kernel/20250425204120.639530125@goodmis.org/

Where one returns true for both PF_KERNEL and PF_USER_WORKER and the
other returns false.

-- Steve

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Jens Axboe]

On 6/25/25 2:50 PM, Steven Rostedt wrote:
> [
>   Adding Peter Zijlstra as he has been telling me to test against
>   PF_KTHREAD instead of current->mm to tell if it is a kernel thread.
>   But that seems to not be enough!
> ]

Not sure I follow - if current->mm is NULL, then it's PF_KTHREAD too.
Unless it's used kthread_use_mm().

PF_USER_WORKER will have current->mm of the user task that it was cloned
from.

> On Wed, 25 Jun 2025 10:23:28 -0600
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 6/24/25 11:07 AM, Steven Rostedt wrote:
>>> On Mon, 23 Jun 2025 19:59:11 +0800
>>> Jiazi Li <jqqlijiazi@gmail.com> wrote:
>>>   
>>>> Tasks with PF_USER_WORKER flag also only run in kernel space,
>>>> so do not trace user stack for these tasks.  
>>>
>>> What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?  
>>
>> One is a kernel thread (eg no mm, etc), the other is basically a user
>> thread. None of them exit to userspace, that's basically the only
>> thing they have in common.
> 
> Was it ever in user space? Because exiting isn't the issue for getting
> a user space stack. If it never was in user space than sure, there's no
> reason to look at the user space stack.

It was never in userspace.

>>> Has all the locations that test for PF_KTHREAD been audited to make
>>> sure that PF_USER_WORKER isn't also needed?  
>>
>> I did when adding it, to the best of my knowledge. But there certainly
>> could still be gaps. Sometimes not easy to see why code checks for
>> PF_KTHREAD in the first place.
>>
>>> I'm working on other code that needs to differentiate between user
>>> tasks and kernel tasks, and having to have multiple flags to test is
>>> becoming quite a burden.  
>>
>> None of them are user tasks, but PF_USER_WORKER does look like a
>> user thread and acts like one, except it wasn't created by eg
>> pthread_create() and it never returns to userspace. When it's done,
>> it's simply reaped.
>>
> 
> I'm assuming that it also never was in user space, which is where we
> don't want to do any user space stack trace.

It was not.

> This looks like more rationale for having a kernel_task() user_task()
> helper functions:
> 
>   https://lore.kernel.org/linux-trace-kernel/20250425204120.639530125@goodmis.org/
> 
> Where one returns true for both PF_KERNEL and PF_USER_WORKER and the
> other returns false.

On vacation right now, but you can just CC me on the next iteration and
I'll take a look.

-- 
Jens Axboe

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Steven Rostedt]

On Wed, 25 Jun 2025 16:30:55 -0600
Jens Axboe <axboe@kernel.dk> wrote:

> On 6/25/25 2:50 PM, Steven Rostedt wrote:
> > [
> >   Adding Peter Zijlstra as he has been telling me to test against
> >   PF_KTHREAD instead of current->mm to tell if it is a kernel thread.
> >   But that seems to not be enough!
> > ]  
> 
> Not sure I follow - if current->mm is NULL, then it's PF_KTHREAD too.
> Unless it's used kthread_use_mm().
> 
> PF_USER_WORKER will have current->mm of the user task that it was cloned
> from.

The suggestion was to use (current->flags & PF_KTHREAD) instead of
!current->mm to determine if a task is a kernel thread or not as we don't
want to do user space stack tracing on kernel threads. Peter said that
because of io threads which have current->mm set, you can't rely on that,
so check the PF_KHTREAD flag instead. This was assuming that io kthreads
had that set too, but apparently it does not and we need to check for
PF_USER_WORKER instead of just PF_KTHREAD.

> 
> > On Wed, 25 Jun 2025 10:23:28 -0600
> > Jens Axboe <axboe@kernel.dk> wrote:
> >   
> >> On 6/24/25 11:07 AM, Steven Rostedt wrote:  
> >>> On Mon, 23 Jun 2025 19:59:11 +0800
> >>> Jiazi Li <jqqlijiazi@gmail.com> wrote:
> >>>     
> >>>> Tasks with PF_USER_WORKER flag also only run in kernel space,
> >>>> so do not trace user stack for these tasks.    
> >>>
> >>> What exactly is the difference between PF_KTHREAD and PF_USER_WORKER?    
> >>
> >> One is a kernel thread (eg no mm, etc), the other is basically a user
> >> thread. None of them exit to userspace, that's basically the only
> >> thing they have in common.  
> > 
> > Was it ever in user space? Because exiting isn't the issue for getting
> > a user space stack. If it never was in user space than sure, there's no
> > reason to look at the user space stack.  
> 
> It was never in userspace.

OK then for user space stack tracing it is the same as a KTHREAD.

> 
> >>> Has all the locations that test for PF_KTHREAD been audited to make
> >>> sure that PF_USER_WORKER isn't also needed?    
> >>
> >> I did when adding it, to the best of my knowledge. But there certainly
> >> could still be gaps. Sometimes not easy to see why code checks for
> >> PF_KTHREAD in the first place.
> >>  
> >>> I'm working on other code that needs to differentiate between user
> >>> tasks and kernel tasks, and having to have multiple flags to test is
> >>> becoming quite a burden.    
> >>
> >> None of them are user tasks, but PF_USER_WORKER does look like a
> >> user thread and acts like one, except it wasn't created by eg
> >> pthread_create() and it never returns to userspace. When it's done,
> >> it's simply reaped.
> >>  
> > 
> > I'm assuming that it also never was in user space, which is where we
> > don't want to do any user space stack trace.  
> 
> It was not.
> 
> > This looks like more rationale for having a kernel_task() user_task()
> > helper functions:
> > 
> >   https://lore.kernel.org/linux-trace-kernel/20250425204120.639530125@goodmis.org/
> > 
> > Where one returns true for both PF_KERNEL and PF_USER_WORKER and the
> > other returns false.  
> 
> On vacation right now, but you can just CC me on the next iteration and
> I'll take a look.
> 

Well, it was sortof NACKED by Ingo, and he started another version, but I
don't know if that is still happening or not.

  https://lore.kernel.org/linux-trace-kernel/aA0pDUDQViCA1hwi@gmail.com/

Although, that patch just looks like its simply adding helper functions for
all the pf flags, but doesn't solve the issue of just testing "Is this a
kernel thread or user thread?"

-- Steve

Re: [PATCH] stacktrace: do not trace user stack for user_worker tasks

[Keith Busch]

On Wed, Jun 25, 2025 at 06:41:44PM -0400, Steven Rostedt wrote:
> On Wed, 25 Jun 2025 16:30:55 -0600
> Jens Axboe <axboe@kernel.dk> wrote:
> 
> > On 6/25/25 2:50 PM, Steven Rostedt wrote:
> > > [
> > >   Adding Peter Zijlstra as he has been telling me to test against
> > >   PF_KTHREAD instead of current->mm to tell if it is a kernel thread.
> > >   But that seems to not be enough!
> > > ]  
> > 
> > Not sure I follow - if current->mm is NULL, then it's PF_KTHREAD too.
> > Unless it's used kthread_use_mm().
> > 
> > PF_USER_WORKER will have current->mm of the user task that it was cloned
> > from.
> 
> The suggestion was to use (current->flags & PF_KTHREAD) instead of
> !current->mm to determine if a task is a kernel thread or not as we don't
> want to do user space stack tracing on kernel threads. Peter said that
> because of io threads which have current->mm set, you can't rely on that,
> so check the PF_KHTREAD flag instead. This was assuming that io kthreads
> had that set too, but apparently it does not and we need to check for
> PF_USER_WORKER instead of just PF_KTHREAD.

If you're interested, here's a discussion with Linus on some fallout
with PF_USER_WORKER threads I stumbled upon a few months ago with no
clear longterm resolution:

  https://lore.kernel.org/kvm/CAHk-=wg4Wm4x9GoUk6M8BhLsrhLj4+n8jA2Kg8XUQF=kxgNL9g@mail.gmail.com/

That was about userspace problems with these PF_USER_WORKER tasks
spawned with vhost rather than anything in kernel, so it's from the
other side of what you're dealing with here. I'm just mentioning it in
case the improvements your considering could be useful for the userspace
side too.