All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, Jeff Layton <jlayton@kernel.org>,
	Lennart Poettering <lennart@poettering.net>,
	Daan De Meyer <daan.j.demeyer@gmail.com>,
	Mike Yuan <me@yhndnzj.com>
Subject: [PATCH] pidfs: cleanup the usage of do_notify_pidfd()
Date: Sun, 23 Mar 2025 18:19:55 +0100	[thread overview]
Message-ID: <20250323171955.GA834@redhat.com> (raw)
In-Reply-To: <20250320-work-pidfs-thread_group-v4-0-da678ce805bf@kernel.org>

If a single-threaded process exits do_notify_pidfd() will be called twice,
from exit_notify() and right after that from do_notify_parent().

1. Change exit_notify() to call do_notify_pidfd() if the exiting task is
   not ptraced and it is not a group leader.

2. Change do_notify_parent() to call do_notify_pidfd() unconditionally.

   If tsk is not ptraced, do_notify_parent() will only be called when it
   is a group-leader and thread_group_empty() is true.

This means that if tsk is ptraced, do_notify_pidfd() will be called from
do_notify_parent() even if tsk is a delay_group_leader(). But this case is
less common, and apart from the unnecessary __wake_up() is harmless.

Granted, this unnecessary __wake_up() can be avoided, but I don't want to
do it in this patch because it's just a consequence of another historical
oddity: we notify the tracer even if !thread_group_empty(), but do_wait()
from debugger can't work until all other threads exit. With or without this
patch we should either eliminate do_notify_parent() in this case, or change
do_wait(WEXITED) to untrace the ptraced delay_group_leader() at least when
ptrace_reparented().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/exit.c   | 8 ++------
 kernel/signal.c | 8 +++-----
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 683766316a3d..d0ebccb9dec0 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -742,12 +742,6 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
 		kill_orphaned_pgrp(tsk->group_leader, NULL);
 
 	tsk->exit_state = EXIT_ZOMBIE;
-	/*
-	 * Ignore thread-group leaders that exited before all
-	 * subthreads did.
-	 */
-	if (!delay_group_leader(tsk))
-		do_notify_pidfd(tsk);
 
 	if (unlikely(tsk->ptrace)) {
 		int sig = thread_group_leader(tsk) &&
@@ -760,6 +754,8 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
 			do_notify_parent(tsk, tsk->exit_signal);
 	} else {
 		autoreap = true;
+		/* untraced sub-thread */
+		do_notify_pidfd(tsk);
 	}
 
 	if (autoreap) {
diff --git a/kernel/signal.c b/kernel/signal.c
index 027ad9e97417..1d8db0dabb71 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2179,11 +2179,9 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
 
 	WARN_ON_ONCE(!tsk->ptrace &&
 	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
-	/*
-	 * Notify for thread-group leaders without subthreads.
-	 */
-	if (thread_group_empty(tsk))
-		do_notify_pidfd(tsk);
+
+	/* ptraced, or group-leader without sub-threads */
+	do_notify_pidfd(tsk);
 
 	if (sig != SIGCHLD) {
 		/*
-- 
2.25.1.362.g51ebf55



  parent reply	other threads:[~2025-03-23 17:20 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-20 13:24 [PATCH v4 0/4] pidfs: handle multi-threaded exec and premature thread-group leader exit Christian Brauner
2025-03-20 13:24 ` [PATCH v4 1/4] pidfs: improve multi-threaded exec and premature thread-group leader exit polling Christian Brauner
2025-03-20 14:28   ` Oleg Nesterov
2025-03-20 15:26     ` Christian Brauner
2025-03-20 13:24 ` [PATCH v4 2/4] selftests/pidfd: first test for multi-threaded exec polling Christian Brauner
2025-03-20 13:24 ` [PATCH v4 3/4] selftests/pidfd: second " Christian Brauner
2025-03-20 13:24 ` [PATCH v4 4/4] selftests/pidfd: third " Christian Brauner
2025-03-23 17:19 ` Oleg Nesterov [this message]
2025-03-23 17:45   ` selftests/pidfd: (Was: [PATCH] pidfs: cleanup the usage of do_notify_pidfd()) Oleg Nesterov
2025-03-23 20:36     ` Christian Brauner
2025-03-23 21:17       ` Oleg Nesterov
2025-03-23 20:40   ` [PATCH] pidfs: cleanup the usage of do_notify_pidfd() Christian Brauner
2025-03-23 20:42   ` Christian Brauner
2025-03-24 17:19 ` [PATCH] exit: fix the usage of delay_group_leader->exit_code in do_notify_parent() and pidfs_exit() Oleg Nesterov
2025-03-25 14:57   ` Christian Brauner

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=20250323171955.GA834@redhat.com \
    --to=oleg@redhat.com \
    --cc=brauner@kernel.org \
    --cc=daan.j.demeyer@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=lennart@poettering.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=me@yhndnzj.com \
    /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.