Skip to contentSkip to conversation

locking: opt-in tracking of sleeping lock holders

PATCH Superseded Shakeel Butt 2 participants 10 messages Sep 23 – Sep 24

Subsystems (10)
lore
Share & tools
lore
#0 lore raw

cover locking: opt-in tracking of sleeping lock holders

This series adds a cheap way to answer one question:

Is this task holding a lock that other tasks may be waiting for?

The answer is a count in task_struct. Only the locks that ask to be tracked are counted. The mechanism introduced here is planned to be used by solutions for shared lock isolation scenarios [1].

The problem
===========

A task that stalls while holding a widely shared lock does not just slow itself down. Everything waiting behind that lock slows down too, and those waiters can be from unrelated cgroups.

This is an isolation failure. Pressure in one workload leaks into an unrelated workload through a shared lock, and the second workload has no way to defend itself.

The case we hit is memory. An allocation made while holding a lock can get stuck in reclaim, either because a memcg limit is being enforced or because the machine is low on memory. If the lock is cgroup_mutex or kernfs_rwsem, every other cgroup operation on the machine waits for that reclaim, whichever cgroup caused it.

Knowing the task holds such a lock lets the allocator treat it differently:

- under a memcg limit, skip the throttling that enforcement does inline, and leave it for the return to user space, where memory.high is enforced anyway;

- under global pressure, let the allocation reach the memory reserves so it finishes instead of waiting for reclaim.

Either way the task gets out of the critical section quickly, and the pressure is still accounted for.

CPU limits have the same shape of problem, and the scheduler has already paid to avoid it: commit e1fad12dcb66 ("sched/fair: Switch to task based throttle model") stopped throttling a task the moment its cpu.max quota runs out, and now defers it to the return to user space, so a task cannot be stopped in the middle of a critical section. That works without knowing anything about locks, because it defers for every task. The allocator cannot do the same, because dropping the inline work for everybody may cause more isolation issues due to global memory pressure, or make the system unstable by fully depleting the memory reserves.

Why not something that already exists
=====================================

Everything in the tree that can answer the question is too expensive, and the ones that are cheap enough cannot answer it.

Too expensive:

- lockdep. lock_is_held_type() answers exactly this, for every sleeping lock type including the rw_semaphore read side. It is the only thing in the tree that does. But the cost cannot be trimmed down to it:
DEBUG_LOCK_ALLOC selects DEBUG_SPINLOCK, DEBUG_MUTEXES and LOCKDEP, and LOCKDEP selects STACKTRACE and KALLSYMS_ALL. Measured here on will-it-scale, x86_64: DEBUG_LOCK_ALLOC costs 36-53% throughput and PROVE_LOCKING 39-70%, and task_struct grows from 4096 to 6784 bytes. lockdep also answers about one named lock, so the caller would have to know the set of interesting locks and test each one.

- BPF on the lock primitives [1]. This works, and we prototyped it:
fentry on the down_*() exits, fexit on the up_*() entries, per task depth in task storage. It cost about 8% throughput against the same policy hooking a single function, because the probe fires on every down_read() in the kernel and only then compares the address.

Cannot answer it:

- The locks' own owner fields. A mutex, an rt_mutex and a write held rw_semaphore know their owner. The rw_semaphore read side does not, and that is the side that matters. kernel/locking/rwsem.c says why:
"Ideally we would like to track all the readers that own a rwsem, but the overhead is simply too big." percpu_rw_semaphore is the same, by
design: its readers are spread over per-CPU counters.

- hung-task blocker tracking. task->blocker records what a task waits for, not what it holds, and inherits the same rwsem reader gap.

- Lock tracepoints. The two that fire on every acquire and release are inside #ifdef CONFIG_LOCKDEP. The ones that need no lockdep only fire on contention. The closest, contended_release, is holder side but fires at release, after the decision has to be made.

- A new PF_ flag set by the callers. Save and restore cannot express non-LIFO release, and sleeping locks are released out of order all the time: take A, take B, release A, and the flag is cleared while B is still held. Getting that right needs a counter, which is this series with hand written call sites. The cgroupfs kernfs rwsems alone are taken in 37 places, and missing one is silent.

- Reusing PF_MEMALLOC. It means "I am reclaiming, let me into the reserves": it grants ALLOC_NO_WATERMARKS and skips direct reclaim. A cgroupfs walker holding kernfs_rwsem across many allocations would drain the reserves rather than politely skip a throttle.

- rwsem_is_locked() and friends say somebody holds the lock, not you.

This series inverts that: instead of a lock knowing its many readers, each task knows how many interesting locks it holds.

The opt-in is a spare bit of a word the lock already has, bit 3 of rw_semaphore::count and of mutex::owner, or existing padding, so on 64-bit no lock grows. With CONFIG_TRACK_LOCK_HOLDERS=n the locking code is unchanged. With it built in but nothing opted in, the hooks are a static branch that has not been patched in, so every lock still runs the instructions it ran before.

The patches
===========

 1  the core: the counter, the static key, the hooks, Kconfig
 2  rw_semaphore support
 3  mutex support
 4  percpu_rw_semaphore support
 5  KUnit tests
 6  documentation
 7  opt in the cgroupfs locks: the kernfs rwsems, cgroup_mutex and

cgroup_threadgroup_rwsem

Testing
=======

19 KUnit tests in kernel/locking/lockholder_kunit.c. Sixteen check that the count always matches the number of opted-in locks the task holds, across every way each lock type can be taken and dropped: fast and slow paths, failed trylocks, out of order release, downgrade_write(), the percpu handover calls, cleanup.h guards, and a second task to check the counts are per task. Three cover what the opt-in changes besides the
count: that an opted-in but unheld lock still reads unlocked, that *_track_holder() refuses a lock that is already held, and that the non_owner rwsem calls warn and do not count.

  tools/testing/kunit/kunit.py run --kunitconfig=kernel/locking/ lockholder

They pass in four configurations: UML, x86_64, x86_64 with PROVE_LOCKING, DEBUG_MUTEXES and DEBUG_RWSEMS, and x86_64 with PREEMPT_RT. The debug one earns its place twice over: it compiles the mutex fast paths out, so every acquire and release goes through the slow paths that rebuild the flag word, and it is the only one that runs the non_owner test, as without CONFIG_DEBUG_LOCK_ALLOC those calls are defined to plain down_read() and up_read(). That test skips in the other three. A full x86_64 build with modules is clean.

[1] https://lore.kernel.org/[email protected]/

Shakeel Butt (7):
locking: add opt-in tracking of sleeping lock holders locking/rwsem: track holders of opted-in rw_semaphores locking/mutex: track holders of opted-in mutexes locking/percpu-rwsem: track holders of opted-in percpu_rw_semaphores locking/selftests: add KUnit tests for lock holder tracking Documentation/locking: document sleeping lock holder tracking kernfs, cgroup: track holders of the cgroupfs locks

 Documentation/locking/index.rst               |   1 +
 .../locking/lock-holder-tracking.rst          | 168 ++++
 MAINTAINERS                                   |   1 +
 fs/kernfs/dir.c                               |   5 +
 include/linux/kernfs.h                        |   7 +
 include/linux/lockholder.h                    | 119 +++
 include/linux/mutex.h                         |  14 +
 include/linux/percpu-rwsem.h                  |  42 +
 include/linux/rtmutex.h                       |  11 +
 include/linux/rwsem.h                         |  65 +-
 include/linux/sched.h                         |   8 +
 kernel/Kconfig.locks                          |  16 +
 kernel/cgroup/cgroup.c                        |  27 +-
 kernel/exit.c                                 |   4 +
 kernel/fork.c                                 |   3 +
 kernel/locking/.kunitconfig                   |   4 +
 kernel/locking/Makefile                       |   2 +
 kernel/locking/lockholder.c                   |  19 +
 kernel/locking/lockholder_kunit.c             | 741 ++++++++++++++++++
 kernel/locking/mutex.c                        |  79 +-
 kernel/locking/mutex.h                        |  48 +-
 kernel/locking/percpu-rwsem.c                 |  28 +
 kernel/locking/rtmutex_api.c                  |  36 +-
 kernel/locking/rtmutex_common.h               |   3 +
 kernel/locking/rwsem.c                        |  96 ++-
 lib/Kconfig.debug                             |  13 +
 26 files changed, 1539 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/locking/lock-holder-tracking.rst
 create mode 100644 include/linux/lockholder.h
 create mode 100644 kernel/locking/.kunitconfig
 create mode 100644 kernel/locking/lockholder.c
 create mode 100644 kernel/locking/lockholder_kunit.c

base-commit: next-20260922

Signature · 1 line
2.47.3
#1 lore raw

PATCH 1/7 locking: add opt-in tracking of sleeping lock holders

A task that stalls while holding a widely shared lock also stalls the tasks waiting for that lock. One source of such stalls is a memory allocation by the lock holder, which can get stuck in memory reclaim because the system is low on memory or because a memcg limit is being enforced. More concretely, if an allocation made while holding cgroup_mutex or kernfs_rwsem gets stuck in memory reclaim (global or memcg), other cgroupfs operations can end up waiting too, even for unrelated cgroups.

To avoid this, the allocation path (memcg and global) needs a cheap way to tell whether the task holds such a lock. Locks do not record all their holders, so ask the task instead: let each task count the sleeping locks it holds that have opted in to tracking. Read the count with task_holds_tracked_lock(), current_holds_tracked_lock() or task_nr_tracked_locks(). BPF can read task->nr_tracked_locks through BTF. Only the task itself changes its count, so it needs no atomics.

No lock is tracked by default. Until the first lock opts in, the hooks are a static branch that is off (a 2-byte NOP on x86_64). The first opt-in turns it on for good.

---
 MAINTAINERS                 |   1 +
 include/linux/lockholder.h  | 119 ++++++++++++++++++++++++++++++++++++
 include/linux/sched.h       |   8 +++
 kernel/Kconfig.locks        |  16 +++++
 kernel/exit.c               |   4 ++
 kernel/fork.c               |   3 +
 kernel/locking/Makefile     |   1 +
 kernel/locking/lockholder.c |  19 ++++++
 8 files changed, 171 insertions(+)
 create mode 100644 include/linux/lockholder.h
 create mode 100644 kernel/locking/lockholder.c
MAINTAINERS +1 -0
index dccddc99ac9a..097e3d9e4973 100644
@@ -15428,6 +15428,7 @@ F:	Documentation/locking/
 F:	arch/*/include/asm/spinlock*.h
 F:	include/linux/local_lock*.h
 F:	include/linux/lockdep*.h
+F:	include/linux/lockholder.h
 F:	include/linux/mutex*.h
 F:	include/linux/rwlock*.h
 F:	include/linux/rwsem*.h
include/linux/lockholder.h new file +119 -0

119 lines not on this page · raw message

include/linux/sched.h +8 -0
index 5ce350a616e4..3f7f459e33fd 100644
@@ -1306,6 +1306,14 @@ struct task_struct {
 	struct held_lock		held_locks[MAX_LOCK_DEPTH];
 #endif
 
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+	/*
+	 * Number of opted-in sleeping locks held. Only the task itself
+	 * changes it, so it needs no atomics. See linux/lockholder.h.
+	 */
+	unsigned int			nr_tracked_locks;
+#endif
+
 #if defined(CONFIG_UBSAN) && !defined(CONFIG_UBSAN_TRAP)
 	unsigned int			in_ubsan;
 #endif
kernel/Kconfig.locks +16 -0
index 1c6423aafcd4..9489e6ce72c0 100644
@@ -279,3 +279,19 @@ config ARCH_HAS_MMIOWB
 config MMIOWB
 	def_bool y if ARCH_HAS_MMIOWB
 	depends on SMP
+
+config TRACK_LOCK_HOLDERS
+	bool "Track holders of selected sleeping locks"
+	help
+	  Keep a per-task count of held sleeping locks that have opted in
+	  to tracking. This lets code check whether a task holds a lock
+	  that others may be waiting for, e.g. to avoid stalling it in
+	  memcg limit enforcement or under global memory pressure.
+
+	  No lock is tracked until one opts in. Until then, the lock hooks
+	  are a static branch that is off. The opt-in is a spare bit in a
+	  word the lock already has, or existing padding, so on 64-bit no
+	  lock grows. Where there is no padding to use, as on 32-bit, some
+	  locks grow by a word.
+
+	  If unsure, say N.
kernel/exit.c +4 -0
index 0430d7c16600..57b05fb19ea3 100644
@@ -35,6 +35,7 @@
 #include <linux/mount.h>
 #include <linux/proc_fs.h>
 #include <linux/kthread.h>
+#include <linux/lockholder.h>
 #include <linux/mempolicy.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
@@ -1025,6 +1026,9 @@ void __noreturn do_exit(long code)
 	 * Make sure we are holding no locks:
 	 */
 	debug_check_no_locks_held();
+	WARN_ONCE(task_holds_tracked_lock(tsk),
+		  "%s/%d exiting while holding %u tracked lock(s)\n",
+		  tsk->comm, task_pid_nr(tsk), task_nr_tracked_locks(tsk));
 
 	if (tsk->io_context)
 		exit_io_context(tsk);
kernel/fork.c +3 -0
index 39be51048d77..6c461c7e2187 100644
@@ -94,6 +94,7 @@
 #include <linux/sysctl.h>
 #include <linux/kcov.h>
 #include <linux/livepatch.h>
+#include <linux/lockholder.h>
 #include <linux/thread_info.h>
 #include <linux/kstack_erase.h>
 #include <linux/kasan.h>
@@ -2265,6 +2266,8 @@ __latent_entropy struct task_struct *copy_process(
 
 	lockdep_init_task(p);
 
+	lock_holder_task_init(p);
+
 	p->blocked_on = NULL; /* not blocked yet */
 	p->blocked_donor = NULL; /* nobody is boosting p yet */
 
kernel/locking/Makefile +1 -0
index cee1901d4cff..a0945beb304c 100644
@@ -9,6 +9,7 @@ CONTEXT_ANALYSIS_ww_rt_mutex.o := y
 CONTEXT_ANALYSIS_rwsem.o := y
 
 obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o
+obj-$(CONFIG_TRACK_LOCK_HOLDERS) += lockholder.o
 
 # Avoid recursion lockdep -> sanitizer -> ... -> lockdep & improve performance.
 KASAN_SANITIZE_lockdep.o := n
kernel/locking/lockholder.c new file +19 -0
new file mode 100644
index 000000000000..99428c6141c6
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Opt-in tracking of sleeping lock holders. See linux/lockholder.h.
+ */
+#include <linux/export.h>
+#include <linux/lockholder.h>
+
+/* Exported for inline lock functions used by modules. */
+DEFINE_STATIC_KEY_FALSE(lock_holder_tracking_key);
+EXPORT_SYMBOL_GPL(lock_holder_tracking_key);
+
+void lock_holder_tracking_enable(void)
+{
+	/* static_branch_enable() takes cpus_read_lock() even if already on. */
+	if (static_key_enabled(&lock_holder_tracking_key))
+		return;
+
+	static_branch_enable(&lock_holder_tracking_key);
+}
Signature · 1 line
2.53.0-Meta
#2 lore raw

PATCH 2/7 locking/rwsem: track holders of opted-in rw_semaphores

In reply to Shakeel Butt

Add rwsem_track_holder() to opt an rw_semaphore in to holder tracking.

The count goes up once down_*() has the lock and down before up_*() releases it. Waiting is not counted, as the task does not hold the lock yet. The killable, interruptible and trylock variants count only on success. downgrade_write() keeps the lock held, so it needs no hook.

down_read_non_owner() and up_read_non_owner() can run in different tasks, which a per-task count cannot follow. Both warn on a tracked rwsem and neither counts. The warn is on both sides because the two are not always paired with each other: bpf task_iter takes mm->mmap_lock with mmap_read_lock_killable() and drops it with up_read_non_owner(), so warning only on the acquire side would miss it. This matters only with CONFIG_DEBUG_LOCK_ALLOC, as without it both are defined to plain down_read() and up_read(), which are counted and balanced.

A lock must opt in before anyone takes it. A task already holding it was not counted, so its up_*() would make its count wrong. rwsem_track_holder() warns and does nothing if the rwsem is held, and __init_rwsem() clears the flag with the rest of count.

The opt-in is bit 3 of ->count, which is reserved. It is sticky: unlike the other flag bits it survives unlock, so a free tracked rwsem reads RWSEM_FLAG_TRACKED rather than zero, and rwsem_is_locked() and rwsem_assert_held_nolockdep() mask it off. Nothing else has to change, because every other path either adds to count or writes back a value it read, so the bit comes along.

PREEMPT_RT builds rw_semaphore on rt_mutex_base, which clears ->owner on unlock, so a bit there would not survive. On 64-bit it has four bytes of padding after ->wait_lock, so the flag goes there.

---
 include/linux/rtmutex.h         | 11 ++++
 include/linux/rwsem.h           | 65 +++++++++++++++++++++-
 kernel/locking/rtmutex_common.h |  3 ++
 kernel/locking/rwsem.c          | 96 ++++++++++++++++++++++++++++++---
 4 files changed, 167 insertions(+), 8 deletions(-)
include/linux/rtmutex.h +11 -0
index 9e1f012f89db..3001a25705ac 100644
@@ -22,6 +22,17 @@ extern int max_lock_depth;
 
 struct rt_mutex_base {
 	raw_spinlock_t		wait_lock;
+#if defined(CONFIG_TRACK_LOCK_HOLDERS) && defined(CONFIG_PREEMPT_RT)
+	/*
+	 * Opt-in to holder tracking, set before first use. On PREEMPT_RT
+	 * the sleeping locks that can be tracked are built on this one.
+	 * On 64-bit the flag fits in the padding after wait_lock, so none
+	 * of them grows. Where that padding does not exist, as on 32-bit,
+	 * everything built on rt_mutex_base grows by a word, spinlock_t
+	 * included.
+	 */
+	bool			tracked;
+#endif
 	struct rb_root_cached   waiters __guarded_by(&wait_lock);
 	struct task_struct	*owner  __guarded_by(&wait_lock);
 };
include/linux/rwsem.h +63 -2

98 lines not on this page · raw message

kernel/locking/rtmutex_common.h +3 -0
index c38b7bdea7b3..2228ac11910a 100644
@@ -180,6 +180,9 @@ enum rtmutex_chainwalk {
 
 static inline void __rt_mutex_base_init(struct rt_mutex_base *lock)
 {
+#if defined(CONFIG_TRACK_LOCK_HOLDERS) && defined(CONFIG_PREEMPT_RT)
+	lock->tracked = false;
+#endif
 	scoped_guard (raw_spinlock_init, &lock->wait_lock) {
 		lock->waiters = RB_ROOT_CACHED;
 		lock->owner = NULL;
kernel/locking/rwsem.c +90 -6

240 lines not on this page · raw message

Signature · 1 line
2.53.0-Meta
#3 lore raw

PATCH 3/7 locking/mutex: track holders of opted-in mutexes

In reply to Shakeel Butt

Add mutex_track_holder() to opt a mutex in to holder tracking.

On !PREEMPT_RT, current takes a mutex only in __mutex_trylock_common() and __mutex_trylock_fast(), and releases it only in __mutex_unlock_fast() and __mutex_unlock_slowpath(). Hooking these four places covers every mutex_lock*(), mutex_trylock(), ww_mutex_lock*() and handoff. __mutex_unlock_fast() drops the count after the release, which needs no care: the count is in current, not in the mutex.

On PREEMPT_RT, the hooks are in __mutex_lock_common(), mutex_trylock(), _mutex_trylock_nest_lock() and mutex_unlock() in rtmutex_api.c. There, ww_mutex is an rt_mutex and is not covered. mutex_track_holder() itself is shared by both builds.

As with rwsem, mutex_track_holder() warns and does nothing if the mutex is held, and mutex_init() clears the flag with the rest of ->owner.

The opt-in is bit 3 of ->owner, which is below the alignment of a task_struct pointer. It is sticky: unlike the other flag bits it survives unlock, so a free tracked mutex reads MUTEX_FLAG_TRACKED rather than zero.

Bit 3 needs a task_struct aligned to 16 rather than the 8 that bits 0-2 needed. fork_init() aligns to at least L1_CACHE_BYTES, whose smallest value in the tree is 16, so a static_assert() records the requirement.

---
 include/linux/mutex.h        | 14 +++++++
 kernel/locking/mutex.c       | 79 ++++++++++++++++++++++++++++++++----
 kernel/locking/mutex.h       | 48 +++++++++++++++++++++-
 kernel/locking/rtmutex_api.c | 36 ++++++++++++++--
 4 files changed, 165 insertions(+), 12 deletions(-)
include/linux/mutex.h +14 -0
index 734048c02f4f..74f54ee83685 100644
@@ -75,6 +75,20 @@ do {									\
  */
 #define mutex_init_with_key(mutex, key) __mutex_init((mutex), #mutex, (key))
 
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+/**
+ * mutex_track_holder - track the holder of a mutex
+ * @lock: initialized mutex that no one holds yet
+ *
+ * Count @lock in task_nr_tracked_locks() of the task holding it. Use it
+ * for locks whose holders can stall unrelated work. Call it before anyone
+ * can take @lock, e.g. right after mutex_init().
+ */
+void mutex_track_holder(struct mutex *lock);
+#else
+static inline void mutex_track_holder(struct mutex *lock) { }
+#endif
+
 #ifndef CONFIG_PREEMPT_RT
 #define __MUTEX_INITIALIZER(lockname) \
 		{ .owner = ATOMIC_LONG_INIT(0) \
kernel/locking/mutex.c +72 -7

151 lines not on this page · raw message

kernel/locking/mutex.h +47 -1

71 lines not on this page · raw message

kernel/locking/rtmutex_api.c +32 -4
index eb18b094473c..41ca3c1d92f6 100644
@@ -2,6 +2,7 @@
 /*
  * rtmutex API
  */
+#include <linux/lockholder.h>
 #include <linux/spinlock.h>
 #include <linux/export.h>
 
@@ -544,10 +545,26 @@ void rt_mutex_debug_task_free(struct task_struct *task)
 /* Mutexes */
 static void __mutex_rt_init_generic(struct mutex *mutex)
 {
+	/* rt_mutex_base_init() also clears the holder tracking opt-in. */
 	rt_mutex_base_init(&mutex->rtmutex);
 	debug_check_no_locks_freed((void *)mutex, sizeof(*mutex));
 }
 
+/*
+ * mutex_track_holder() is in mutex.c, shared with !PREEMPT_RT. Here the
+ * opt-in is a flag in the padding inside the rtmutex, not a bit of
+ * ->owner: rt_mutex clears ->owner on unlock, so a bit there would not
+ * survive.
+ */
+static inline bool mutex_is_tracked(struct mutex *lock)
+{
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+	return lock->rtmutex.tracked;
+#else
+	return false;
+#endif
+}
+
 static __always_inline int __mutex_lock_common(struct mutex *lock,
 					       unsigned int state,
 					       unsigned int subclass,
@@ -560,10 +577,12 @@ static __always_inline int __mutex_lock_common(struct mutex *lock,
 	might_sleep();
 	mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
 	ret = __rt_mutex_lock(&lock->rtmutex, state);
-	if (ret)
+	if (ret) {
 		mutex_release(&lock->dep_map, ip);
-	else
+	} else {
 		lock_acquired(&lock->dep_map, ip);
+		lock_holder_acquired_if(mutex_is_tracked(lock));
+	}
 	return ret;
 }
 
@@ -623,8 +642,10 @@ int __sched _mutex_trylock_nest_lock(struct mutex *lock,
 		return 0;
 
 	ret = __rt_mutex_trylock(&lock->rtmutex);
-	if (ret)
+	if (ret) {
 		mutex_acquire_nest(&lock->dep_map, 0, 1, nest_lock, _RET_IP_);
+		lock_holder_acquired_if(mutex_is_tracked(lock));
+	}
 
 	return ret;
 }
@@ -666,10 +687,16 @@ EXPORT_SYMBOL(mutex_lock_io);
 
 int __sched mutex_trylock(struct mutex *lock)
 {
+	int ret;
+
 	if (IS_ENABLED(CONFIG_DEBUG_RT_MUTEXES) && WARN_ON_ONCE(!in_task()))
 		return 0;
 
-	return __rt_mutex_trylock(&lock->rtmutex);
+	ret = __rt_mutex_trylock(&lock->rtmutex);
+	if (ret)
+		lock_holder_acquired_if(mutex_is_tracked(lock));
+
+	return ret;
 }
 EXPORT_SYMBOL(mutex_trylock);
 #endif /* !CONFIG_DEBUG_LOCK_ALLOC */
@@ -677,6 +704,7 @@ EXPORT_SYMBOL(mutex_trylock);
 void __sched mutex_unlock(struct mutex *lock)
 	__releases(lock) __no_context_analysis
 {
+	lock_holder_released_if(mutex_is_tracked(lock));
 	mutex_release(&lock->dep_map, _RET_IP_);
 	__rt_mutex_unlock(&lock->rtmutex);
 }
Signature · 1 line
2.53.0-Meta
#4 lore raw

PATCH 4/7 locking/percpu-rwsem: track holders of opted-in percpu_rw_semaphores

In reply to Shakeel Butt

Add percpu_rwsem_track_holder() to opt a percpu_rw_semaphore in to holder tracking.

The read fast and slow paths meet before the hook in percpu_down_read_internal(), so one hook covers both. percpu_down_read_trylock() counts only on success.

percpu_rwsem_release() and percpu_rwsem_acquire() pass a held lock to another task. Filesystem freeze uses them to return to user space holding the lock, and async writes (kiocb_start_write()) use them because the write may complete in another context. Move the count in these calls too, so it stays right for each task.

The read side counts inside the preempt-disabled region, so a task is never preemptible holding @sem without being counted for it.

---
 include/linux/percpu-rwsem.h  | 42 +++++++++++++++++++++++++++++++++++
 kernel/locking/percpu-rwsem.c | 28 +++++++++++++++++++++++
 2 files changed, 70 insertions(+)
include/linux/percpu-rwsem.h +42 -0
index 39d5bf8e6562..1a131ed8e8f1 100644
@@ -8,6 +8,7 @@
 #include <linux/wait.h>
 #include <linux/rcu_sync.h>
 #include <linux/lockdep.h>
+#include <linux/lockholder.h>
 #include <linux/cleanup.h>
 
 struct percpu_rw_semaphore {
@@ -16,6 +17,13 @@ struct percpu_rw_semaphore {
 	struct rcuwait		writer;
 	wait_queue_head_t	waiters;
 	atomic_t		block;
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+	/*
+	 * Set by percpu_rwsem_track_holder() before first use. Fits in the
+	 * padding after block, so the struct does not grow on 64-bit.
+	 */
+	bool			tracked;
+#endif
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lockdep_map	dep_map;
 #endif
@@ -65,6 +73,11 @@ static inline void percpu_down_read_internal(struct percpu_rw_semaphore *sem,
 		this_cpu_inc(*sem->read_count);
 	else
 		__percpu_down_read(sem, false, freezable); /* Unconditional memory barrier */
+	/*
+	 * Count the lock before preemption comes back, so the task is
+	 * never preemptible holding @sem but not counted for it.
+	 */
+	lock_holder_acquired(sem);
 	/*
 	 * The preempt_enable() prevents the compiler from
 	 * bleeding the critical section out.
@@ -95,6 +108,12 @@ static inline bool percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
 		this_cpu_inc(*sem->read_count);
 	else
 		ret = __percpu_down_read(sem, true, false); /* Unconditional memory barrier */
+	/*
+	 * Count the lock before preemption comes back, as in
+	 * percpu_down_read_internal().
+	 */
+	if (ret)
+		lock_holder_acquired(sem);
 	preempt_enable();
 	/*
 	 * The barrier() from preempt_enable() prevents the compiler from
@@ -111,6 +130,7 @@ extern void __percpu_up_read(struct percpu_rw_semaphore *sem);
 
 static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
 {
+	lock_holder_released(sem);
 	rwsem_release(&sem->dep_map, _RET_IP_);
 
 	preempt_disable();
@@ -129,6 +149,20 @@ extern bool percpu_is_read_locked(struct percpu_rw_semaphore *);
 extern void percpu_down_write(struct percpu_rw_semaphore *);
 extern void percpu_up_write(struct percpu_rw_semaphore *);
 
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+/**
+ * percpu_rwsem_track_holder - track the holders of a percpu_rw_semaphore
+ * @sem: initialized percpu_rw_semaphore that no one holds yet
+ *
+ * Count @sem in task_nr_tracked_locks() of the tasks holding it. Use it
+ * for locks whose holders can stall unrelated work. Call it before anyone
+ * can take @sem, e.g. right after percpu_init_rwsem().
+ */
+void percpu_rwsem_track_holder(struct percpu_rw_semaphore *sem);
+#else
+static inline void percpu_rwsem_track_holder(struct percpu_rw_semaphore *sem) { }
+#endif
+
 DEFINE_GUARD(percpu_read, struct percpu_rw_semaphore *,
 	     percpu_down_read(_T), percpu_up_read(_T))
 DEFINE_GUARD_COND(percpu_read, _try, percpu_down_read_trylock(_T))
@@ -156,9 +190,16 @@ extern void percpu_free_rwsem(struct percpu_rw_semaphore *);
 #define percpu_rwsem_is_held(sem)	lockdep_is_held(sem)
 #define percpu_rwsem_assert_held(sem)	lockdep_assert_held(sem)
 
+/*
+ * These pass a held lock to another task. For example, filesystem freeze
+ * returns to user space holding the lock, and thaw, maybe in another
+ * task, releases it. The holder count moves with the lock, as lockdep's
+ * view of it does.
+ */
 static inline void percpu_rwsem_release(struct percpu_rw_semaphore *sem,
 					unsigned long ip)
 {
+	lock_holder_released(sem);
 	lock_release(&sem->dep_map, ip);
 }
 
@@ -166,6 +207,7 @@ static inline void percpu_rwsem_acquire(struct percpu_rw_semaphore *sem,
 					bool read, unsigned long ip)
 {
 	lock_acquire(&sem->dep_map, 0, 1, read, 1, NULL, ip);
+	lock_holder_acquired(sem);
 }
 
 #endif
kernel/locking/percpu-rwsem.c +28 -0
index 6c78961fe753..e100c33731ca 100644
@@ -22,6 +22,9 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *sem,
 	rcuwait_init(&sem->writer);
 	init_waitqueue_head(&sem->waiters);
 	atomic_set(&sem->block, 0);
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+	sem->tracked = false;
+#endif
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	debug_check_no_locks_freed((void *)sem, sizeof(*sem));
 	lockdep_init_map(&sem->dep_map, name, key, 0);
@@ -201,6 +204,28 @@ bool percpu_is_read_locked(struct percpu_rw_semaphore *sem)
 }
 EXPORT_SYMBOL_GPL(percpu_is_read_locked);
 
+#ifdef CONFIG_TRACK_LOCK_HOLDERS
+void percpu_rwsem_track_holder(struct percpu_rw_semaphore *sem)
+{
+	/*
+	 * A task holding @sem now was not counted, so its percpu_up_*()
+	 * would make its count wrong.
+	 */
+	if (WARN_ONCE(percpu_is_read_locked(sem) || percpu_is_write_locked(sem),
+		      "%s: rwsem is held; opt in before it is published\n",
+		      __func__))
+		return;
+
+	/*
+	 * Turn the hooks on before the flag, so that a lock carrying the
+	 * flag always has them on.
+	 */
+	lock_holder_tracking_enable();
+	sem->tracked = true;
+}
+EXPORT_SYMBOL_GPL(percpu_rwsem_track_holder);
+#endif
+
 /*
  * Return true if the modular sum of the sem->read_count per-CPU variable is
  * zero.  If this sum is zero, then it is stable due to the fact that if any
@@ -256,11 +281,14 @@ void __sched percpu_down_write(struct percpu_rw_semaphore *sem)
 	rcuwait_wait_event(&sem->writer, readers_active_check(sem), TASK_UNINTERRUPTIBLE);
 	if (contended)
 		trace_contention_end(sem, 0);
+
+	lock_holder_acquired(sem);
 }
 EXPORT_SYMBOL_GPL(percpu_down_write);
 
 void percpu_up_write(struct percpu_rw_semaphore *sem)
 {
+	lock_holder_released(sem);
 	rwsem_release(&sem->dep_map, _RET_IP_);
 
 	if (trace_contended_release_enabled() && wq_has_sleeper(&sem->waiters))
Signature · 1 line
2.53.0-Meta
#5 lore raw

PATCH 5/7 locking/selftests: add KUnit tests for lock holder tracking

In reply to Shakeel Butt

Add 16 KUnit tests that check task_nr_tracked_locks(current) always equals the number of opted-in locks the task holds. They cover:

- locks that did not opt in, which must not count
- the mutex, rw_semaphore and percpu_rw_semaphore lock and trylock calls, including the killable, interruptible, freezable and io ones
- downgrade_write(), the cleanup.h guards, and nested locks released out of order
- the percpu_rwsem_release()/percpu_rwsem_acquire() hand-off
- re-init clearing the opt-in

Five tests use a helper thread that holds a lock. They check that a failed trylock does not count, that a lock taken right as the helper releases it (usually through the slow path) counts exactly once, and that each task has its own count.

Three cases cover what the opt-in changes besides the count: that an opted-in but unheld lock still reads unlocked, which the sticky bit would otherwise break for rwsem_is_locked() and its callers; that *_track_holder() refuses a lock that is already held; and that the non_owner rwsem calls warn and do not count. The last needs CONFIG_DEBUG_LOCK_ALLOC, as without it they are plain down_read() and up_read(), so it skips otherwise.

Tests compare against the count at the start of the test, not zero.

Passes on UML, x86_64, x86_64 with PROVE_LOCKING + DEBUG_MUTEXES + DEBUG_RWSEMS (where the mutex fast path is compiled out), and x86_64 PREEMPT_RT:

  tools/testing/kunit/kunit.py run --kunitconfig=kernel/locking/ lockholder
---
 kernel/locking/.kunitconfig       |   4 +
 kernel/locking/Makefile           |   1 +
 kernel/locking/lockholder_kunit.c | 741 ++++++++++++++++++++++++++++++
 lib/Kconfig.debug                 |  13 +
 4 files changed, 759 insertions(+)
 create mode 100644 kernel/locking/.kunitconfig
 create mode 100644 kernel/locking/lockholder_kunit.c
kernel/locking/.kunitconfig new file +4 -0
new file mode 100644
index 000000000000..ed243843b8b0
@@ -0,0 +1,4 @@
+CONFIG_KUNIT=y
+CONFIG_SMP=y
+CONFIG_TRACK_LOCK_HOLDERS=y
+CONFIG_TRACK_LOCK_HOLDERS_KUNIT_TEST=y
kernel/locking/Makefile +1 -0
index a0945beb304c..d23b754df40f 100644
@@ -10,6 +10,7 @@ CONTEXT_ANALYSIS_rwsem.o := y
 
 obj-y += mutex.o semaphore.o rwsem.o percpu-rwsem.o
 obj-$(CONFIG_TRACK_LOCK_HOLDERS) += lockholder.o
+obj-$(CONFIG_TRACK_LOCK_HOLDERS_KUNIT_TEST) += lockholder_kunit.o
 
 # Avoid recursion lockdep -> sanitizer -> ... -> lockdep & improve performance.
 KASAN_SANITIZE_lockdep.o := n
kernel/locking/lockholder_kunit.c new file +741 -0

741 lines not on this page · raw message

lib/Kconfig.debug +13 -0
index 163192b2ed7f..67fc86b4fc6f 100644
@@ -1672,6 +1672,19 @@ config WW_MUTEX_SELFTEST
 	  Say M if you want these self tests to build as a module.
 	  Say N if you are unsure.
 
+config TRACK_LOCK_HOLDERS_KUNIT_TEST
+	tristate "KUnit tests for sleeping lock holder tracking" if !KUNIT_ALL_TESTS
+	depends on KUNIT && TRACK_LOCK_HOLDERS
+	default KUNIT_ALL_TESTS
+	help
+	  Check that task_nr_tracked_locks() always equals the number of
+	  opted-in locks the task holds.
+
+	  For more information on KUnit and unit tests in general, refer to
+	  the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config SCF_TORTURE_TEST
 	tristate "torture tests for smp_call_function*()"
 	depends on DEBUG_KERNEL
Signature · 1 line
2.53.0-Meta
#6 lore raw

PATCH 6/7 Documentation/locking: document sleeping lock holder tracking

In reply to Shakeel Butt

Describe why the count exists, how a lock opts in, what it costs, where each lock type updates the count, and what is not covered.

---
 Documentation/locking/index.rst               |   1 +
 .../locking/lock-holder-tracking.rst          | 168 ++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100644 Documentation/locking/lock-holder-tracking.rst
Documentation/locking/index.rst +1 -0
index 9278d95b7dcb..6593c924f9a7 100644
@@ -9,6 +9,7 @@ Locking
 
     locktypes
     lockdep-design
+    lock-holder-tracking
     lockstat
     locktorture
     mutex-design
Documentation/locking/lock-holder-tracking.rst new file +168 -0

168 lines not on this page · raw message

Signature · 1 line
2.53.0-Meta
#7 lore raw

PATCH 7/7 kernfs, cgroup: track holders of the cgroupfs locks

In reply to Shakeel Butt

Opt the cgroupfs kernfs rwsems, cgroup_mutex and cgroup_threadgroup_rwsem in to lock holder tracking.

Add KERNFS_ROOT_TRACK_LOCK_HOLDERS, which opts in a root's kernfs_rwsem, kernfs_iattr_rwsem and kernfs_supers_rwsem, and pass it from cgroup_setup_root(). Other kernfs users have their own roots, so only cgroupfs is affected. All three are held across work that can allocate, so a holder stuck in reclaim can stall many other cgroupfs operations; kernfs_iattr_rwsem is write-held across kernfs_iattrs(), which allocates with GFP_KERNEL.

cgroup_mutex is taken by every cgroup create, destroy and migrate, and is held across the same kind of work, so a stalled holder stalls the whole cgroupfs control plane.

cgroup_threadgroup_rwsem is taken for read on every fork and exit, and usually for write on migration, so a stalled holder can stall process creation on the whole machine.

The last two are statically defined, so opt them in from cgroup_init(). That runs after jump_label_init() and before anything can take either lock.

signal->cgroup_threadgroup_rwsem, used with favordynmods, is left alone. Its readers also hold cgroup_threadgroup_rwsem, so they are counted already, and its writer only blocks the process being migrated.

These opt-ins are for a policy that reads the count. On their own they only turn on the static branch, so they should go in with that policy.

---
 fs/kernfs/dir.c        |  5 +++++
 include/linux/kernfs.h |  7 +++++++
 kernel/cgroup/cgroup.c | 27 ++++++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)
fs/kernfs/dir.c +5 -0
index cc6288d5b4cc..1116bb38c1ff 100644
@@ -1035,6 +1035,11 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
 	init_rwsem(&root->kernfs_rwsem);
 	init_rwsem(&root->kernfs_iattr_rwsem);
 	init_rwsem(&root->kernfs_supers_rwsem);
+	if (flags & KERNFS_ROOT_TRACK_LOCK_HOLDERS) {
+		rwsem_track_holder(&root->kernfs_rwsem);
+		rwsem_track_holder(&root->kernfs_iattr_rwsem);
+		rwsem_track_holder(&root->kernfs_supers_rwsem);
+	}
 	INIT_LIST_HEAD(&root->supers);
 	rwlock_init(&root->kernfs_rename_lock);
 
include/linux/kernfs.h +7 -0
index 73786b567ccd..ea2efdc5277e 100644
@@ -156,6 +156,13 @@ enum kernfs_root_flag {
 	 * Renames must not change the parent node.
 	 */
 	KERNFS_ROOT_INVARIANT_PARENT		= 0x0010,
+
+	/*
+	 * Track the holders of this root's rwsems (see
+	 * CONFIG_TRACK_LOCK_HOLDERS).  Use it for roots whose locks are
+	 * shared so widely that a stalled holder stalls unrelated work.
+	 */
+	KERNFS_ROOT_TRACK_LOCK_HOLDERS		= 0x0020,
 };
 
 /* type-specific structures for kernfs_node union members */
kernel/cgroup/cgroup.c +26 -1
index 804318ae160e..87d66aae2558 100644
@@ -2191,11 +2191,18 @@ int cgroup_setup_root(struct cgroup_root *root, u32 ss_mask)
 	kf_sops = root == &cgrp_dfl_root ?
 		&cgroup_kf_syscall_ops : &cgroup1_kf_syscall_ops;
 
+	/*
+	 * Track the holders of the cgroupfs kernfs locks.  They are held
+	 * across work that can charge memory, so a holder stuck in reclaim
+	 * can stall many other cgroupfs operations.  Other kernfs users have
+	 * their own roots and are not affected.
+	 */
 	root->kf_root = kernfs_create_root(kf_sops,
 					   KERNFS_ROOT_CREATE_DEACTIVATED |
 					   KERNFS_ROOT_SUPPORT_EXPORTOP |
 					   KERNFS_ROOT_SUPPORT_USER_XATTR |
-					   KERNFS_ROOT_INVARIANT_PARENT,
+					   KERNFS_ROOT_INVARIANT_PARENT |
+					   KERNFS_ROOT_TRACK_LOCK_HOLDERS,
 					   root_cgrp);
 	if (IS_ERR(root->kf_root)) {
 		ret = PTR_ERR(root->kf_root);
@@ -6552,6 +6559,24 @@ int __init cgroup_init(void)
 	get_user_ns(init_cgroup_ns.user_ns);
 	cgroup_rt_init();
 
+	/*
+	 * cgroup_mutex is taken by every cgroup create, destroy and migrate
+	 * and is held across work that can allocate, so a holder stuck in
+	 * reclaim stalls the whole cgroupfs control plane.
+	 *
+	 * cgroup_threadgroup_rwsem is taken for read by fork and exit, and
+	 * usually for write by migration, so a stalled holder can stall
+	 * process creation on the whole machine.
+	 *
+	 * Neither can be held yet, as rest_init() has not run.
+	 *
+	 * signal->cgroup_threadgroup_rwsem is not tracked.  Its readers hold
+	 * cgroup_threadgroup_rwsem too, so they are counted already, and its
+	 * writer only blocks the process being migrated.
+	 */
+	mutex_track_holder(&cgroup_mutex);
+	percpu_rwsem_track_holder(&cgroup_threadgroup_rwsem);
+
 	cgroup_lock();
 
 	/*
Signature · 1 line
2.53.0-Meta
maintainer
#8 lore raw
In reply to Shakeel Butt
Quoted 8 lines · Shakeel Butt

Shakeel Butt (7):
locking: add opt-in tracking of sleeping lock holders locking/rwsem: track holders of opted-in rw_semaphores locking/mutex: track holders of opted-in mutexes locking/percpu-rwsem: track holders of opted-in percpu_rw_semaphores locking/selftests: add KUnit tests for lock holder tracking Documentation/locking: document sleeping lock holder tracking kernfs, cgroup: track holders of the cgroupfs locks

Not sure what's gone wrong, but I only have a partial set. If you want me to look at them, send me whole set.

#9 lore raw
Quoted 11 lines · Peter Zijlstra
Quoted 8 lines · Shakeel Butt

Shakeel Butt (7):
locking: add opt-in tracking of sleeping lock holders locking/rwsem: track holders of opted-in rw_semaphores locking/mutex: track holders of opted-in mutexes locking/percpu-rwsem: track holders of opted-in percpu_rw_semaphores locking/selftests: add KUnit tests for lock holder tracking Documentation/locking: document sleeping lock holder tracking kernfs, cgroup: track holders of the cgroupfs locks

Quoted 2 lines

Not sure what's gone wrong, but I only have a partial set. If you want me to look at them, send me whole set.

Thanks Peter for taking a look. I will resend soon (couple of hours).