mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-kernel@vger.kernel.org
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [PATCH 1/2] signal: Add proper comment about the preempt-disable in ptrace_stop().
Date: Thu,  3 Aug 2023 12:09:31 +0200	[thread overview]
Message-ID: <20230803100932.325870-2-bigeasy@linutronix.de> (raw)
In-Reply-To: <20230803100932.325870-1-bigeasy@linutronix.de>

Commit 53da1d9456fe7 ("fix ptrace slowness") added a preempt-disable section
between read_unlock() and the following schedule() invocation without
explaining why it is needed.

Replace the comment with an explanation why this is needed. Clarify that
it is needed for correctness but for performance reasons.

Acked-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/signal.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2318,10 +2318,21 @@ static int ptrace_stop(int exit_code, in
 		do_notify_parent_cldstop(current, false, why);
 
 	/*
-	 * Don't want to allow preemption here, because
-	 * sys_ptrace() needs this task to be inactive.
+	 * The previous do_notify_parent_cldstop() invocation woke ptracer.
+	 * One a PREEMPTION kernel this can result in preemption requirement
+	 * which will be fulfilled after read_unlock() and the ptracer will be
+	 * put on the CPU.
+	 * The ptracer is in wait_task_inactive(, __TASK_TRACED) waiting for
+	 * this task wait in schedule(). If this task gets preempted then it
+	 * remains enqueued on the runqueue. The ptracer will observe this and
+	 * then sleep for a delay of one HZ tick. In the meantime this task
+	 * gets scheduled, enters schedule() and will wait for the ptracer.
 	 *
-	 * XXX: implement read_unlock_no_resched().
+	 * This preemption point is not bad from correctness point of view but
+	 * extends the runtime by one HZ tick time due to the ptracer's sleep.
+	 * The preempt-disable section ensures that there will be no preemption
+	 * between unlock and schedule() and so improving the performance since
+	 * the ptracer has no reason to sleep.
 	 */
 	preempt_disable();
 	read_unlock(&tasklist_lock);

  reply	other threads:[~2023-08-03 10:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-03 10:09 [PATCH REPOST v3 0/2] signal: Avoid preempt_disable() in ptrace_stop() on PREEMPT_RT Sebastian Andrzej Siewior
2023-08-03 10:09 ` Sebastian Andrzej Siewior [this message]
2023-09-19 20:16   ` [tip: core/core] signal: Add a proper comment about preempt_disable() in ptrace_stop() tip-bot2 for Sebastian Andrzej Siewior
2023-09-20  6:57     ` Ingo Molnar
2023-09-20  7:47       ` [PATCH] signal: Correct spelling typos " Sebastian Andrzej Siewior
2023-08-03 10:09 ` [PATCH 2/2] signal: Don't disable preemption in ptrace_stop() on PREEMPT_RT Sebastian Andrzej Siewior
2023-09-19 20:16   ` [tip: core/core] " tip-bot2 for Sebastian Andrzej Siewior
2023-08-17 13:46 ` [PATCH REPOST v3 0/2] signal: Avoid preempt_disable() " Sebastian Andrzej Siewior
  -- strict thread matches above, loose matches on Subject: below --
2023-04-06 20:57 [PATCH " Sebastian Andrzej Siewior
2023-04-06 20:57 ` [PATCH 1/2] signal: Add proper comment about the preempt-disable in ptrace_stop() Sebastian Andrzej Siewior

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=20230803100932.325870-2-bigeasy@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®