[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.22.394.2504261157210.3375@hadrien>
Date: Sat, 26 Apr 2025 12:41:23 +0200 (CEST)
From: Julia Lawall <julia.lawall@...ia.fr>
To: Andrew Morton <akpm@...ux-foundation.org>
cc: Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, Masami Hiramatsu <mhiramat@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Peter Zijlstra <peterz@...radead.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
Kees Cook <kees@...nel.org>, bpf@...r.kernel.org,
Tejun Heo <tj@...nel.org>, Julia Lawall <Julia.Lawall@...ia.fr>,
Nicolas Palix <nicolas.palix@...g.fr>, cocci@...ia.fr
Subject: Re: [RFC][PATCH 0/2] Add is_user_thread() and is_kernel_thread()
helper functions
On Fri, 25 Apr 2025, Andrew Morton wrote:
> On Fri, 25 Apr 2025 16:41:20 -0400 Steven Rostedt <rostedt@...dmis.org> wrote:
>
> > While working on the deferred stacktrace code, Peter Zijlstra told
> > me to use task->flags & PF_KTHREAD instead of checking task->mm for NULL.
> > This seemed reasonable, but while working on it, as there were several
> > places that check if the task is a kernel thread and other places that
> > check if the task is a user space thread I found it a bit confusing
> > when looking at both:
> >
> > if (task->flags & PF_KTHREAD)
> > and
> > if (!(task->flags & PF_KTHREAD))
> >
> > Where I mixed them up sometimes, and checked for a user space thread when I
> > really wanted to check for a kernel thread. I found these mistakes before
> > sending out my patches, but going back and reviewing the code, I always had
> > to stop and spend a few unnecessary seconds making sure the check was
> > testing that flag correctly.
> >
> > To make this a bit more obvious, I introduced two helper functions:
> >
> > is_user_thread(task)
> > is_kernel_thread(task)
> >
> > which simply test the flag for you. Thus, seeing:
> >
> > if (is_user_thread(task))
> > or
> > if (is_kernel_thread(task))
> >
> > it was very obvious to which test you wanted to make.
>
> Seems sensible. Please consider renaming PF_KTHREAD in order to break
> missed conversion sites.
Maybe:
@r depends on !(file in "include/linux/sched.h")@ // Kees's suggestion
position p;
expression e;
@@
(
e = (PF_KTHREAD | ...)
|
e |= (PF_KTHREAD | ...)
|
PF_KTHREAD@p
)
@script:ocaml@ // change to python if desired
p << r.p;
@@
Printf.printf "%s:%d: Warning: remaining use of PF_KTHREAD\n" (List.hd p).file (List.hd p).line
julia
Powered by blists - more mailing lists