signal: Add a proper comment about preempt_disable() in ptrace_stop()
authorSebastian Andrzej Siewior <bigeasy@linutronix.de>
Thu, 3 Aug 2023 10:09:31 +0000 (12:09 +0200)
committerThomas Gleixner <tglx@linutronix.de>
Tue, 19 Sep 2023 20:08:29 +0000 (22:08 +0200)
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 existing contentless comment with a proper explanation to
clarify that it is not needed for correctness but for performance reasons.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20230803100932.325870-2-bigeasy@linutronix.de
kernel/signal.c

index 0901901..3035beb 100644 (file)
@@ -2329,10 +2329,22 @@ static int ptrace_stop(int exit_code, int why, unsigned long message,
                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 a 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 will observe that
+        * the tracee is scheduled out once it gets on the CPU.
         */
        preempt_disable();
        read_unlock(&tasklist_lock);