All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: rjw@rjwysocki.net, oleg@redhat.com, mingo@kernel.org,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, mgorman@suse.de,
	Will Deacon <will@kernel.org>
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
	tj@kernel.org, linux-pm@vger.kernel.org
Subject: [PATCH v3 5/6] sched,ptrace: Avoid relying on __TASK_TRACED | __TASK_STOPPED
Date: Sat, 09 Oct 2021 12:07:59 +0200	[thread overview]
Message-ID: <20211009101445.090696671@infradead.org> (raw)
In-Reply-To: <20211009100754.690769957@infradead.org>

Make ->ptrace/->jobctl the canonical state, this allows us to play
games with __state (such as freezing).

The wait_task_inactive() usage will be fixed up later, once we have
additional TASK_state.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/sched.h |   27 ++++++++++++++++++++++-----
 kernel/ptrace.c       |   16 +++++++++-------
 2 files changed, 31 insertions(+), 12 deletions(-)

--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -118,11 +118,9 @@ struct task_group;
 
 #define task_is_running(task)		(READ_ONCE((task)->__state) == TASK_RUNNING)
 
-#define task_is_traced(task)		((READ_ONCE(task->__state) & __TASK_TRACED) != 0)
-
-#define task_is_stopped(task)		((READ_ONCE(task->__state) & __TASK_STOPPED) != 0)
-
-#define task_is_stopped_or_traced(task)	((READ_ONCE(task->__state) & (__TASK_STOPPED | __TASK_TRACED)) != 0)
+#define task_is_traced(task)		((task)->ptrace & PT_STOPPED)
+#define task_is_stopped(task)		((task)->jobctl & JOBCTL_STOP_PENDING)
+#define task_is_stopped_or_traced(task)	(task_is_stopped(task) || task_is_traced(task))
 
 /*
  * Special states are those that do not use the normal wait-loop pattern. See
@@ -228,6 +226,25 @@ struct task_group;
 	} while (0)
 
 /*
+ * task_cond_set_special_state() is a cmpxchg like operation on task->state.
+ *
+ * This operation isn't safe in general and should only be used to transform
+ * one (special) blocked state into another, such as:
+ *   TASK_STOPPED <-> TASK_FROZEN.
+ */
+#define task_cond_set_special_state(task, cond_state)			\
+	({								\
+		struct task_struct *__p = (task);			\
+		unsigned long __flags; /* may shadow */			\
+		unsigned int __state;					\
+		raw_spin_lock_irqsave(&__p->pi_lock, __flags);		\
+		if ((__state = (cond_state)))				\
+			WRITE_ONCE(__p->__state, __state);		\
+		raw_spin_unlock_irqrestore(&__p->pi_lock, __flags);	\
+		!!__state;						\
+	})
+
+/*
  * PREEMPT_RT specific variants for "sleeping" spin/rwlocks
  *
  * RT's spin/rwlock substitutions are state preserving. The state of the
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -207,7 +207,8 @@ static bool ptrace_freeze_traced(struct
 	    !__fatal_signal_pending(task)) {
 		task->ptrace &= ~PT_STOPPED_MASK;
 		task->ptrace |= PT_STOPPED;
-		WRITE_ONCE(task->__state, __TASK_TRACED);
+		/* *TASK_TRACED -> __TASK_TRACED */
+		task_cond_set_special_state(task, !!(task->__state & __TASK_TRACED) * __TASK_TRACED);
 		ret = true;
 	}
 	spin_unlock_irq(&task->sighand->siglock);
@@ -217,7 +218,7 @@ static bool ptrace_freeze_traced(struct
 
 static void ptrace_unfreeze_traced(struct task_struct *task)
 {
-	if (READ_ONCE(task->__state) != __TASK_TRACED)
+	if (!task_is_traced(task))
 		return;
 
 	WARN_ON(!task->ptrace || task->parent != current);
@@ -227,13 +228,14 @@ static void ptrace_unfreeze_traced(struc
 	 * Recheck state under the lock to close this race.
 	 */
 	spin_lock_irq(&task->sighand->siglock);
-	if (READ_ONCE(task->__state) == __TASK_TRACED) {
+	if (task_is_traced(task)) {
 		if (__fatal_signal_pending(task)) {
 			task->ptrace &= ~PT_STOPPED_MASK;
 			wake_up_state(task, __TASK_TRACED);
 		} else {
 			task->ptrace |= PT_STOPPED_MASK;
-			WRITE_ONCE(task->__state, TASK_TRACED);
+			/* *TASK_TRACED -> TASK_TRACED */
+			task_cond_set_special_state(task, !!(task->__state & __TASK_TRACED) * TASK_TRACED);
 		}
 	}
 	spin_unlock_irq(&task->sighand->siglock);
@@ -269,7 +271,7 @@ static int ptrace_check_attach(struct ta
 	 */
 	read_lock(&tasklist_lock);
 	if (child->ptrace && child->parent == current) {
-		WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED);
+//		WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED);
 		/*
 		 * child->sighand can't be NULL, release_task()
 		 * does ptrace_unlink() before __exit_signal().
@@ -280,13 +282,13 @@ static int ptrace_check_attach(struct ta
 	read_unlock(&tasklist_lock);
 
 	if (!ret && !ignore_state) {
-		if (!wait_task_inactive(child, __TASK_TRACED)) {
+		if (!wait_task_inactive(child, __TASK_TRACED)) { // XXX mooo!!!
 			/*
 			 * This can only happen if may_ptrace_stop() fails and
 			 * ptrace_stop() changes ->state back to TASK_RUNNING,
 			 * so we should not worry about leaking __TASK_TRACED.
 			 */
-			WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED);
+//			WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED);
 			ret = -ESRCH;
 		}
 	}



  parent reply	other threads:[~2021-10-09 10:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-09 10:07 [PATCH v3 0/6] Freezer rewrite Peter Zijlstra
2021-10-09 10:07 ` [PATCH v3 1/6] freezer: Have {,un}lock_system_sleep() save/restore flags Peter Zijlstra
2021-10-14  8:58   ` Will Deacon
2021-10-09 10:07 ` [PATCH v3 2/6] freezer,umh: Clean up freezer/initrd interaction Peter Zijlstra
2021-10-14  9:13   ` Will Deacon
2021-10-09 10:07 ` [PATCH v3 3/6] ptrace: Order and comment PT_flags Peter Zijlstra
2021-10-14  9:31   ` Will Deacon
2021-10-14 14:27     ` Peter Zijlstra
2021-10-09 10:07 ` [PATCH v3 4/6] ptrace: Track __TASK_TRACED state in p->ptrace Peter Zijlstra
2021-10-09 10:07 ` Peter Zijlstra [this message]
2021-10-09 10:08 ` [PATCH v3 6/6] freezer,sched: Rewrite core freezer logic Peter Zijlstra
2021-10-18 13:36   ` Peter Zijlstra

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=20211009101445.090696671@infradead.org \
    --to=peterz@infradead.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=rjw@rjwysocki.net \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=will@kernel.org \
    --subject='Re: [PATCH v3 5/6] sched,ptrace: Avoid relying on __TASK_TRACED | __TASK_STOPPED' \
    /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

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.