All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@inria.fr>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,
	 linux-trace-kernel@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	 Peter Zijlstra <peterz@infradead.org>,
	 Linus Torvalds <torvalds@linux-foundation.org>,
	 Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org,  Kees Cook <kees@kernel.org>,
	bpf@vger.kernel.org,  Tejun Heo <tj@kernel.org>,
	Julia Lawall <Julia.Lawall@inria.fr>,
	 Nicolas Palix <nicolas.palix@imag.fr>,
	cocci@inria.fr
Subject: Re: [RFC][PATCH 0/2] Add is_user_thread() and is_kernel_thread() helper functions
Date: Sat, 26 Apr 2025 12:41:23 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2504261157210.3375@hadrien> (raw)
In-Reply-To: <20250425161449.7a2516b3fe0d5de3e2d2b677@linux-foundation.org>



On Fri, 25 Apr 2025, Andrew Morton wrote:

> On Fri, 25 Apr 2025 16:41:20 -0400 Steven Rostedt <rostedt@goodmis.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


WARNING: multiple messages have this Message-ID (diff)
From: Julia Lawall <julia.lawall@inria.fr>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,
	 linux-trace-kernel@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	 Peter Zijlstra <peterz@infradead.org>,
	 Linus Torvalds <torvalds@linux-foundation.org>,
	 Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org,  Kees Cook <kees@kernel.org>,
	bpf@vger.kernel.org,  Tejun Heo <tj@kernel.org>,
	Julia Lawall <Julia.Lawall@inria.fr>,
	 Nicolas Palix <nicolas.palix@imag.fr>,
	cocci@inria.fr
Subject: Re: [cocci] [RFC][PATCH 0/2] Add is_user_thread() and is_kernel_thread() helper functions
Date: Sat, 26 Apr 2025 12:41:23 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2504261157210.3375@hadrien> (raw)
In-Reply-To: <20250425161449.7a2516b3fe0d5de3e2d2b677@linux-foundation.org>



On Fri, 25 Apr 2025, Andrew Morton wrote:

> On Fri, 25 Apr 2025 16:41:20 -0400 Steven Rostedt <rostedt@goodmis.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


  reply	other threads:[~2025-04-26 10:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25 20:41 [RFC][PATCH 0/2] Add is_user_thread() and is_kernel_thread() helper functions Steven Rostedt
2025-04-25 20:41 ` [RFC][PATCH 1/2] kthread: " Steven Rostedt
2025-04-25 23:03   ` Kees Cook
2025-04-26 12:36     ` Steven Rostedt
2025-04-26 11:08   ` Borislav Petkov
2025-04-26 12:37     ` Steven Rostedt
2025-04-25 20:41 ` [RFC][PATCH 2/2] treewide: Have the task->flags & PF_KTHREAD check use the " Steven Rostedt
2025-04-25 23:09   ` Kees Cook
2025-04-26  3:22     ` Alexei Starovoitov
2025-04-28 18:34       ` Tejun Heo
2025-04-25 23:14 ` [RFC][PATCH 0/2] Add is_user_thread() and is_kernel_thread() " Andrew Morton
2025-04-26 10:41   ` Julia Lawall [this message]
2025-04-26 10:41     ` [cocci] " Julia Lawall
2025-04-26 12:43   ` Steven Rostedt
2025-04-26 18:42     ` [PATCH] sched/core: Introduce task_*() helpers for PF_ flags Ingo Molnar
2025-04-26 18:51       ` Ingo Molnar
2025-04-26 20:06       ` Steven Rostedt
2025-04-28 12:12       ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.22.394.2504261157210.3375@hadrien \
    --to=julia.lawall@inria.fr \
    --cc=akpm@linux-foundation.org \
    --cc=bpf@vger.kernel.org \
    --cc=cocci@inria.fr \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=nicolas.palix@imag.fr \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.