All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <jstultz@google.com>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>,
	Joel Fernandes <joelaf@google.com>,
	Qais Yousef <qyousef@google.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Valentin Schneider <vschneid@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>,
	Zimuzo Ezeozue <zezeozue@google.com>,
	Youssef Esmat <youssefesmat@google.com>,
	Mel Gorman <mgorman@suse.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	kernel-team@android.com, "Connor O'Brien" <connoro@google.com>,
	John Stultz <jstultz@google.com>
Subject: [PATCH v6 07/20] locking/mutex: Add p->blocked_on wrappers for correctness checks
Date: Mon,  6 Nov 2023 19:34:50 +0000	[thread overview]
Message-ID: <20231106193524.866104-8-jstultz@google.com> (raw)
In-Reply-To: <20231106193524.866104-1-jstultz@google.com>

From: Valentin Schneider <valentin.schneider@arm.com>

This lets us assert p->blocked_lock is held whenever we access
p->blocked_on, as well as warn us for unexpected state changes.

Cc: Joel Fernandes <joelaf@google.com>
Cc: Qais Yousef <qyousef@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ben Segall <bsegall@google.com>
Cc: Zimuzo Ezeozue <zezeozue@google.com>
Cc: Youssef Esmat <youssefesmat@google.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Waiman Long <longman@redhat.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: "Paul E . McKenney" <paulmck@kernel.org>
Cc: kernel-team@android.com
Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
[fix conflicts, call in more places]
Signed-off-by: Connor O'Brien <connoro@google.com>
[jstultz: tweaked commit subject, added get_task_blocked_on() as well]
Signed-off-by: John Stultz <jstultz@google.com>
---
v2:
* Added get_task_blocked_on() accessor
v4:
* Address READ_ONCE usage that was dropped in v2
* Reordered to be a later add on to the main patch series as
  Peter was unhappy with similar wrappers in other patches.
v5:
* Added some extra correctness checking in wrappers
---
 include/linux/sched.h        | 22 ++++++++++++++++++++++
 kernel/locking/mutex-debug.c |  4 ++--
 kernel/locking/mutex.c       | 10 +++++-----
 kernel/locking/ww_mutex.h    |  4 ++--
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a9258dae00e0..81334677e008 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2248,6 +2248,28 @@ static inline int rwlock_needbreak(rwlock_t *lock)
 #endif
 }
 
+static inline void set_task_blocked_on(struct task_struct *p, struct mutex *m)
+{
+	lockdep_assert_held(&p->blocked_lock);
+
+	/* We should be setting values to NULL or NULL to values */
+	WARN_ON((!m && !p->blocked_on) || (m && p->blocked_on));
+
+	p->blocked_on = m;
+}
+
+static inline struct mutex *get_task_blocked_on(struct task_struct *p)
+{
+	lockdep_assert_held(&p->blocked_lock);
+
+	return p->blocked_on;
+}
+
+static inline struct mutex *get_task_blocked_on_once(struct task_struct *p)
+{
+	return READ_ONCE(p->blocked_on);
+}
+
 static __always_inline bool need_resched(void)
 {
 	return unlikely(tif_need_resched());
diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c
index 7228909c3e62..1eedf7c60c00 100644
--- a/kernel/locking/mutex-debug.c
+++ b/kernel/locking/mutex-debug.c
@@ -53,13 +53,13 @@ void debug_mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 	lockdep_assert_held(&lock->wait_lock);
 
 	/* Current thread can't be already blocked (since it's executing!) */
-	DEBUG_LOCKS_WARN_ON(task->blocked_on);
+	DEBUG_LOCKS_WARN_ON(get_task_blocked_on(task));
 }
 
 void debug_mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 			 struct task_struct *task)
 {
-	struct mutex *blocked_on = READ_ONCE(task->blocked_on);
+	struct mutex *blocked_on = get_task_blocked_on_once(task);
 
 	DEBUG_LOCKS_WARN_ON(list_empty(&waiter->list));
 	DEBUG_LOCKS_WARN_ON(waiter->task != task);
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index df186c0bf4a9..36e563f69705 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -623,7 +623,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 			goto err_early_kill;
 	}
 
-	current->blocked_on = lock;
+	set_task_blocked_on(current, lock);
 	set_current_state(state);
 	trace_contention_begin(lock, LCB_F_MUTEX);
 	for (;;) {
@@ -670,7 +670,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 		/*
 		 * Gets reset by unlock path().
 		 */
-		current->blocked_on = lock;
+		set_task_blocked_on(current, lock);
 		set_current_state(state);
 		/*
 		 * Here we order against unlock; we must either see it change
@@ -699,7 +699,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 		}
 	}
 acquired:
-	current->blocked_on = NULL;
+	set_task_blocked_on(current, NULL);
 	__set_current_state(TASK_RUNNING);
 
 	if (ww_ctx) {
@@ -731,7 +731,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 	return 0;
 
 err:
-	current->blocked_on = NULL;
+	set_task_blocked_on(current, NULL);
 	__set_current_state(TASK_RUNNING);
 	__mutex_remove_waiter(lock, &waiter);
 err_early_kill:
@@ -950,7 +950,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
 		debug_mutex_wake_waiter(lock, waiter);
 		raw_spin_lock(&next->blocked_lock);
 		WARN_ON(next->blocked_on != lock);
-		next->blocked_on = NULL;
+		set_task_blocked_on(current, NULL);
 		raw_spin_unlock(&next->blocked_lock);
 		wake_q_add(&wake_q, next);
 	}
diff --git a/kernel/locking/ww_mutex.h b/kernel/locking/ww_mutex.h
index 2929a95b4272..44a532dda927 100644
--- a/kernel/locking/ww_mutex.h
+++ b/kernel/locking/ww_mutex.h
@@ -292,7 +292,7 @@ __ww_mutex_die(struct MUTEX *lock, struct MUTEX_WAITER *waiter,
 		 * blocked_on relationships that can't resolve.
 		 */
 		WARN_ON(waiter->task->blocked_on != lock);
-		waiter->task->blocked_on = NULL;
+		set_task_blocked_on(waiter->task, NULL);
 		wake_q_add(wake_q, waiter->task);
 		raw_spin_unlock(&waiter->task->blocked_lock);
 	}
@@ -349,7 +349,7 @@ static bool __ww_mutex_wound(struct MUTEX *lock,
 			 * blocked_on pointer. Otherwise we can see circular
 			 * blocked_on relationships that can't resolve.
 			 */
-			owner->blocked_on = NULL;
+			set_task_blocked_on(owner, NULL);
 			wake_q_add(wake_q, owner);
 			raw_spin_unlock(&owner->blocked_lock);
 		}
-- 
2.42.0.869.gea05f2083d-goog


  parent reply	other threads:[~2023-11-06 19:36 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-06 19:34 [PATCH v6 00/20] Proxy Execution: A generalized form of Priority Inheritance v6 John Stultz
2023-11-06 19:34 ` [PATCH v6 01/20] sched: Unify runtime accounting across classes John Stultz
2023-12-17 16:19   ` Qais Yousef
2023-12-18 20:23     ` John Stultz
2023-12-28 16:21       ` Qais Yousef
2023-11-06 19:34 ` [PATCH v6 02/20] locking/mutex: Removes wakeups from under mutex::wait_lock John Stultz
2023-11-06 19:34 ` [PATCH v6 03/20] locking/mutex: make mutex::wait_lock irq safe John Stultz
2023-11-06 19:34 ` [PATCH v6 04/20] locking/mutex: Expose __mutex_owner() John Stultz
2023-11-06 19:34 ` [PATCH v6 05/20] locking/mutex: Rework task_struct::blocked_on John Stultz
2023-11-06 19:34 ` [PATCH v6 06/20] locking/mutex: Add task_struct::blocked_lock to serialize changes to the blocked_on state John Stultz
2023-11-06 19:34 ` John Stultz [this message]
2023-11-06 19:34 ` [PATCH v6 08/20] sched: Add CONFIG_PROXY_EXEC & boot argument to enable/disable John Stultz
2023-11-06 19:34 ` [PATCH v6 09/20] locking/mutex: Split blocked_on logic into two states (blocked_on and blocked_on_waking) John Stultz
2023-11-06 19:34 ` [PATCH v6 10/20] locking/mutex: Switch to mutex handoffs for CONFIG_PROXY_EXEC John Stultz
2023-11-06 19:34 ` [PATCH v6 11/20] sched: Split scheduler execution context John Stultz
2023-11-11  9:34   ` kernel test robot
2023-11-11 10:25   ` kernel test robot
2023-11-06 19:34 ` [PATCH v6 12/20] sched: Fix runtime accounting w/ split exec & sched contexts John Stultz
2023-11-11 11:26   ` kernel test robot
2023-11-06 19:34 ` [PATCH v6 13/20] sched: Split out __sched() deactivate task logic into a helper John Stultz
2023-11-06 19:34 ` [PATCH v6 14/20] sched: Add a very simple proxy() function John Stultz
2023-11-11 13:32   ` kernel test robot
2023-11-06 19:34 ` [PATCH v6 15/20] sched: Add proxy deactivate helper John Stultz
2023-11-08  2:51   ` kernel test robot
2023-11-18  0:27     ` John Stultz
2023-11-06 19:34 ` [PATCH v6 16/20] sched: Fix proxy/current (push,pull)ability John Stultz
2023-11-06 19:35 ` [PATCH v6 17/20] sched: Start blocked_on chain processing in proxy() John Stultz
2023-11-06 19:35 ` [PATCH v6 18/20] sched: Handle blocked-waiter migration (and return migration) John Stultz
2023-11-09  5:31   ` Xuewen Yan
2023-11-09  6:08     ` John Stultz
2023-11-09  6:38       ` Xuewen Yan
2023-11-10  3:45         ` John Stultz
2023-11-06 19:35 ` [PATCH v6 19/20] sched: Add blocked_donor link to task for smarter mutex handoffs John Stultz
2023-11-06 19:35 ` [PATCH v6 20/20] sched: Add deactivated (sleeping) owner handling to proxy() John Stultz
     [not found] ` <20231108111458.1368-1-hdanton@sina.com>
2023-11-08 22:13   ` [PATCH v6 00/20] Proxy Execution: A generalized form of Priority Inheritance v6 John Stultz
2023-11-10  9:07 ` Xuewen Yan
2023-12-13  6:37 ` K Prateek Nayak
2023-12-13 16:20   ` Metin Kaya
2023-12-13 19:11   ` John Stultz
2023-12-14  5:15     ` K Prateek Nayak
2023-12-14  1:00   ` John Stultz
2023-12-14  1:03     ` John Stultz
2023-12-17  3:07 ` Qais Yousef
2023-12-18 23:38   ` John Stultz
2023-12-28 16:45     ` Qais Yousef

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=20231106193524.866104-8-jstultz@google.com \
    --to=jstultz@google.com \
    --cc=boqun.feng@gmail.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=connoro@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joelaf@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qyousef@google.com \
    --cc=rostedt@goodmis.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=will@kernel.org \
    --cc=youssefesmat@google.com \
    --cc=zezeozue@google.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.