From: Peter Zijlstra <peterz@infradead.org>
To: Qi Zheng <zhengqi.arch@bytedance.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
"Zhang, Qiang1" <qiang1.zhang@intel.com>,
Boqun Feng <boqun.feng@gmail.com>,
"42.hyeyoo@gmail.com" <42.hyeyoo@gmail.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"roman.gushchin@linux.dev" <roman.gushchin@linux.dev>,
"iamjoonsoo.kim@lge.com" <iamjoonsoo.kim@lge.com>,
"rientjes@google.com" <rientjes@google.com>,
"penberg@kernel.org" <penberg@kernel.org>,
"cl@linux.com" <cl@linux.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Zhao Gongyi <zhaogongyi@bytedance.com>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Thomas Gleixner <tglx@linutronix.de>, RCU <rcu@vger.kernel.org>,
"Paul E . McKenney" <paulmck@kernel.org>
Subject: [PATCH] debugobjects,locking: Annotate __debug_object_init() wait type violation
Date: Sat, 29 Apr 2023 12:06:14 +0200 [thread overview]
Message-ID: <20230429100614.GA1489784@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <6582d31f-e4d5-a982-4797-c22b2dd8b150@bytedance.com>
On Tue, Apr 25, 2023 at 11:51:05PM +0800, Qi Zheng wrote:
> I just tested the following code and
> it can resolve the warning I encountered. :)
Excellent; I've spruiced up the code with a few comments and helpers
(as suggested by Vlastimil on IRC) and will queue the below commit.
---
Subject: debugobjects,locking: Annotate __debug_object_init() wait type violation
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 25 Apr 2023 17:03:13 +0200
There is an explicit wait-type violation in __debug_object_init() for
PREEMPT_RT=n kernels which allows them to more easily fill the object
pool and reduce the chance of allocation failures.
Lockdep's wait-type checks are designed to check the PREEMPT_RT
locking rules even for PREEMPT_RT=n kernels and object to this, so
create a lockdep annotation to allow this to stand.
Specifically, create a 'lock' type that overrides the inner wait-type
while it is held -- allowing one to temporarily raise it, such that
the violation is hidden.
Reported-by: Vlastimil Babka <vbabka@suse.cz>
Reported-by: Qi Zheng <zhengqi.arch@bytedance.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
include/linux/lockdep.h | 14 ++++++++++++++
include/linux/lockdep_types.h | 1 +
kernel/locking/lockdep.c | 28 +++++++++++++++++++++-------
lib/debugobjects.c | 15 +++++++++++++--
4 files changed, 49 insertions(+), 9 deletions(-)
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -339,6 +339,16 @@ extern void lock_unpin_lock(struct lockd
#define lockdep_repin_lock(l,c) lock_repin_lock(&(l)->dep_map, (c))
#define lockdep_unpin_lock(l,c) lock_unpin_lock(&(l)->dep_map, (c))
+/*
+ * Must use lock_map_aquire_try() with override maps to avoid
+ * lockdep thinking they participate in the block chain.
+ */
+#define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \
+ struct lockdep_map _name = { \
+ .name = #_name "-wait-type-override", \
+ .wait_type_inner = _wait_type, \
+ .lock_type = LD_LOCK_WAIT_OVERRIDE, }
+
#else /* !CONFIG_LOCKDEP */
static inline void lockdep_init_task(struct task_struct *task)
@@ -427,6 +437,9 @@ extern int lockdep_is_held(const void *)
#define lockdep_repin_lock(l, c) do { (void)(l); (void)(c); } while (0)
#define lockdep_unpin_lock(l, c) do { (void)(l); (void)(c); } while (0)
+#define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \
+ struct lockdep_map __maybe_unused _name = {}
+
#endif /* !LOCKDEP */
enum xhlock_context_t {
@@ -551,6 +564,7 @@ do { \
#define rwsem_release(l, i) lock_release(l, i)
#define lock_map_acquire(l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
+#define lock_map_acquire_try(l) lock_acquire_exclusive(l, 0, 1, NULL, _THIS_IP_)
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
#define lock_map_acquire_tryread(l) lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)
#define lock_map_release(l) lock_release(l, _THIS_IP_)
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -33,6 +33,7 @@ enum lockdep_wait_type {
enum lockdep_lock_type {
LD_LOCK_NORMAL = 0, /* normal, catch all */
LD_LOCK_PERCPU, /* percpu */
+ LD_LOCK_WAIT_OVERRIDE, /* annotation */
LD_LOCK_MAX,
};
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2253,6 +2253,9 @@ static inline bool usage_match(struct lo
static inline bool usage_skip(struct lock_list *entry, void *mask)
{
+ if (entry->class->lock_type == LD_LOCK_NORMAL)
+ return false;
+
/*
* Skip local_lock() for irq inversion detection.
*
@@ -2279,14 +2282,16 @@ static inline bool usage_skip(struct loc
* As a result, we will skip local_lock(), when we search for irq
* inversion bugs.
*/
- if (entry->class->lock_type == LD_LOCK_PERCPU) {
- if (DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG))
- return false;
+ if (entry->class->lock_type == LD_LOCK_PERCPU &&
+ DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG))
+ return false;
- return true;
- }
+ /*
+ * Skip WAIT_OVERRIDE for irq inversion detection -- it's not actually
+ * a lock and only used to override the wait_type.
+ */
- return false;
+ return true;
}
/*
@@ -4752,7 +4757,8 @@ static int check_wait_context(struct tas
for (; depth < curr->lockdep_depth; depth++) {
struct held_lock *prev = curr->held_locks + depth;
- u8 prev_inner = hlock_class(prev)->wait_type_inner;
+ struct lock_class *class = hlock_class(prev);
+ u8 prev_inner = class->wait_type_inner;
if (prev_inner) {
/*
@@ -4762,6 +4768,14 @@ static int check_wait_context(struct tas
* Also due to trylocks.
*/
curr_inner = min(curr_inner, prev_inner);
+
+ /*
+ * Allow override for annotations -- this is typically
+ * only valid/needed for code that only exists when
+ * CONFIG_PREEMPT_RT=n.
+ */
+ if (unlikely(class->lock_type == LD_LOCK_WAIT_OVERRIDE))
+ curr_inner = prev_inner;
}
}
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -563,10 +563,21 @@ __debug_object_init(void *addr, const st
/*
* On RT enabled kernels the pool refill must happen in preemptible
- * context:
+ * context -- for !RT kernels we rely on the fact that spinlock_t and
+ * raw_spinlock_t are basically the same type and this lock-type
+ * inversion works just fine.
*/
- if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible())
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) {
+ /*
+ * Annotate away the spinlock_t inside raw_spinlock_t warning
+ * by temporarily raising the wait-type to WAIT_SLEEP, matching
+ * the preemptible() condition above.
+ */
+ static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_SLEEP);
+ lock_map_acquire_try(&fill_pool_map);
fill_pool();
+ lock_map_release(&fill_pool_map);
+ }
db = get_bucket((unsigned long) addr);
next prev parent reply other threads:[~2023-04-29 10:06 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-11 13:08 [PATCH] mm: slub: annotate kmem_cache_node->list_lock as raw_spinlock Qi Zheng
2023-04-11 13:40 ` Vlastimil Babka
2023-04-11 14:08 ` Qi Zheng
2023-04-11 14:19 ` Vlastimil Babka
2023-04-11 14:25 ` Qi Zheng
2023-04-12 5:51 ` Boqun Feng
2023-04-12 6:44 ` Zhang, Qiang1
2023-04-12 6:50 ` Vlastimil Babka
2023-04-12 7:30 ` Qi Zheng
2023-04-12 8:32 ` Qi Zheng
2023-04-12 13:09 ` Waiman Long
2023-04-12 16:47 ` Qi Zheng
2023-04-12 12:48 ` Peter Zijlstra
2023-04-12 12:47 ` Peter Zijlstra
2023-04-12 16:44 ` Qi Zheng
2023-04-13 7:40 ` Peter Zijlstra
2023-04-25 15:03 ` Peter Zijlstra
2023-04-25 15:51 ` Qi Zheng
2023-04-29 10:06 ` Peter Zijlstra [this message]
2023-04-13 14:49 ` Qi Zheng
2023-04-12 6:57 ` Qi Zheng
2023-04-13 0:24 ` Joel Fernandes
2023-04-13 1:55 ` Steven Rostedt
2023-04-12 6:45 ` Qi Zheng
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=20230429100614.GA1489784@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=42.hyeyoo@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=bigeasy@linutronix.de \
--cc=boqun.feng@gmail.com \
--cc=cl@linux.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=paulmck@kernel.org \
--cc=penberg@kernel.org \
--cc=qiang1.zhang@intel.com \
--cc=rcu@vger.kernel.org \
--cc=rientjes@google.com \
--cc=roman.gushchin@linux.dev \
--cc=tglx@linutronix.de \
--cc=vbabka@suse.cz \
--cc=zhaogongyi@bytedance.com \
--cc=zhengqi.arch@bytedance.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).