linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] Provide lockdep tracking for bit spin locks
@ 2021-04-09  2:51 Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 01/17] x86: Rename split_lock_init to sld_init Matthew Wilcox (Oracle)
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Since bit spin locks are only a single bit, there's nowhere to put
a lockdep map.  Fortunately, all bit spin locks in the kernel are
conceptually in only a few classes of lock, and by introducing the
split_lock as somewhere to store the lockdep_map, we can track each bit
spin lock's dependencies.

This split_lock would also give us somewhere to queue waiters, should we
choose to do that.  Or a centralised place to handle PREEMPT_RT mutexes.
But I'll leave that for someone who knows what they're doing; for now
this keeps the same implementation.

Matthew Wilcox (Oracle) (17):
  x86: Rename split_lock_init to sld_init
  locking: Add split_lock
  bit_spinlock: Prepare for split_locks
  hlist_bl: Prepare for split_locks
  dm-snap: Add dm_exceptional_lock
  dcache: Add d_hash_lock
  fscache: Add cookie_hash_lock
  gfs2: Add qd_hash_lock
  mbcache: Add mb_cache_lock
  hlist_bl: Make the split_lock parameter mandatory
  s390: Add airq_iv_lock
  zram: Add zram_table_lock
  jbd2: Add jbd2_jh_lock
  slub: Add slab_page_lock
  zsmalloc: Add zs_pin_lock
  rhashtable: Convert to split_lock
  bit_spinlock: Track bit spin locks with lockdep

 arch/s390/include/asm/airq.h  |  5 +++--
 arch/x86/kernel/cpu/intel.c   |  6 +++---
 drivers/block/zram/zram_drv.c |  8 ++++---
 drivers/md/dm-snap.c          | 10 +++++----
 drivers/s390/cio/airq.c       |  3 +++
 fs/dcache.c                   | 25 +++++++++++-----------
 fs/fscache/cookie.c           | 13 ++++++------
 fs/gfs2/quota.c               |  5 +++--
 fs/jbd2/journal.c             | 18 +++++++++-------
 fs/mbcache.c                  | 25 +++++++++++-----------
 include/linux/bit_spinlock.h  | 39 ++++++++++++++++++++++++++++++-----
 include/linux/jbd2.h          | 10 +++++----
 include/linux/list_bl.h       | 15 ++++++++++----
 include/linux/rhashtable.h    | 20 +++++++-----------
 include/linux/split_lock.h    | 37 +++++++++++++++++++++++++++++++++
 lib/rhashtable.c              |  5 +----
 mm/slub.c                     |  6 ++++--
 mm/zsmalloc.c                 | 11 +++++++---
 18 files changed, 174 insertions(+), 87 deletions(-)
 create mode 100644 include/linux/split_lock.h

-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 01/17] x86: Rename split_lock_init to sld_init
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 02/17] locking: Add split_lock Matthew Wilcox (Oracle)
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

I want to use split_lock_init() for a global symbol, so rename this
local one.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 arch/x86/kernel/cpu/intel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fe0bec14d7ec..44ed490c8a49 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -602,7 +602,7 @@ static void init_intel_misc_features(struct cpuinfo_x86 *c)
 	wrmsrl(MSR_MISC_FEATURES_ENABLES, msr);
 }
 
-static void split_lock_init(void);
+static void sld_init(void);
 static void bus_lock_init(void);
 
 static void init_intel(struct cpuinfo_x86 *c)
@@ -720,7 +720,7 @@ static void init_intel(struct cpuinfo_x86 *c)
 	if (tsx_ctrl_state == TSX_CTRL_DISABLE)
 		tsx_disable();
 
-	split_lock_init();
+	sld_init();
 	bus_lock_init();
 
 	intel_init_thermal(c);
@@ -1080,7 +1080,7 @@ static void sld_update_msr(bool on)
 	wrmsrl(MSR_TEST_CTRL, test_ctrl_val);
 }
 
-static void split_lock_init(void)
+static void sld_init(void)
 {
 	if (cpu_model_supports_sld)
 		split_lock_verify_msr(sld_state != sld_off);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 02/17] locking: Add split_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 01/17] x86: Rename split_lock_init to sld_init Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-12 14:29   ` Thomas Gleixner
  2021-04-09  2:51 ` [PATCH 03/17] bit_spinlock: Prepare for split_locks Matthew Wilcox (Oracle)
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Bitlocks do not currently participate in lockdep.  Conceptually, a
bit_spinlock is a split lock, eg across each bucket in a hash table.
The struct split_lock gives us somewhere to record the lockdep_map.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/split_lock.h | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 include/linux/split_lock.h

diff --git a/include/linux/split_lock.h b/include/linux/split_lock.h
new file mode 100644
index 000000000000..8d64cfa0bbad
--- /dev/null
+++ b/include/linux/split_lock.h
@@ -0,0 +1,37 @@
+#ifndef _LINUX_SPLIT_LOCK_H
+#define _LINUX_SPLIT_LOCK_H
+
+#include <linux/lockdep_types.h>
+
+struct split_lock {
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct lockdep_map dep_map;
+#endif
+};
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+#define SPLIT_DEP_MAP_INIT(lockname)					\
+	.dep_map = {							\
+		.name = #lockname,					\
+		.wait_type_inner = LD_WAIT_SPIN,			\
+	}
+#else
+#define SPLIT_DEP_MAP_INIT(lockname)
+#endif
+
+#define DEFINE_SPLIT_LOCK(name)						\
+struct split_lock name = {						\
+	SPLIT_DEP_MAP_INIT(name)					\
+};
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+#define split_lock_init(_lock) do {					\
+	static struct lock_class_key __key;				\
+	lockdep_init_map_wait(&(_lock)->dep_map, #_lock, &__key, 0,	\
+				LD_WAIT_SPIN);				\
+} while (0)
+#else
+static inline void split_lock_init(struct split_lock *sl) { }
+#endif
+
+#endif /* _LINUX_SPLIT_LOCK_H */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 03/17] bit_spinlock: Prepare for split_locks
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 01/17] x86: Rename split_lock_init to sld_init Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 02/17] locking: Add split_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09 14:32   ` Theodore Ts'o
  2021-04-09  2:51 ` [PATCH 04/17] hlist_bl: " Matthew Wilcox (Oracle)
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Make bit_spin_lock() and variants variadic to help with the transition.
The split_lock parameter will become mandatory at the end of the series.
Also add bit_spin_lock_nested() and bit_spin_unlock_assign() which will
both be used by the rhashtable code later.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/bit_spinlock.h | 43 ++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/include/linux/bit_spinlock.h b/include/linux/bit_spinlock.h
index bbc4730a6505..6c5bbb55b334 100644
--- a/include/linux/bit_spinlock.h
+++ b/include/linux/bit_spinlock.h
@@ -6,6 +6,7 @@
 #include <linux/preempt.h>
 #include <linux/atomic.h>
 #include <linux/bug.h>
+#include <linux/split_lock.h>
 
 /*
  *  bit-based spin_lock()
@@ -13,7 +14,8 @@
  * Don't use this unless you really need to: spin_lock() and spin_unlock()
  * are significantly faster.
  */
-static inline void bit_spin_lock(int bitnum, unsigned long *addr)
+static inline void bit_spin_lock_nested(int bitnum, unsigned long *addr,
+		struct split_lock *lock, unsigned int subclass)
 {
 	/*
 	 * Assuming the lock is uncontended, this never enters
@@ -35,10 +37,27 @@ static inline void bit_spin_lock(int bitnum, unsigned long *addr)
 	__acquire(bitlock);
 }
 
+static inline void bit_spin_lock(int bitnum, unsigned long *addr,
+		...)
+{
+	preempt_disable();
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+	while (unlikely(test_and_set_bit_lock(bitnum, addr))) {
+		preempt_enable();
+		do {
+			cpu_relax();
+		} while (test_bit(bitnum, addr));
+		preempt_disable();
+	}
+#endif
+	__acquire(bitlock);
+}
+
 /*
  * Return true if it was acquired
  */
-static inline int bit_spin_trylock(int bitnum, unsigned long *addr)
+static inline int bit_spin_trylock(int bitnum, unsigned long *addr,
+		...)
 {
 	preempt_disable();
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
@@ -54,7 +73,8 @@ static inline int bit_spin_trylock(int bitnum, unsigned long *addr)
 /*
  *  bit-based spin_unlock()
  */
-static inline void bit_spin_unlock(int bitnum, unsigned long *addr)
+static inline void bit_spin_unlock(int bitnum, unsigned long *addr,
+		...)
 {
 #ifdef CONFIG_DEBUG_SPINLOCK
 	BUG_ON(!test_bit(bitnum, addr));
@@ -71,7 +91,8 @@ static inline void bit_spin_unlock(int bitnum, unsigned long *addr)
  *  non-atomic version, which can be used eg. if the bit lock itself is
  *  protecting the rest of the flags in the word.
  */
-static inline void __bit_spin_unlock(int bitnum, unsigned long *addr)
+static inline void __bit_spin_unlock(int bitnum, unsigned long *addr,
+		...)
 {
 #ifdef CONFIG_DEBUG_SPINLOCK
 	BUG_ON(!test_bit(bitnum, addr));
@@ -83,6 +104,20 @@ static inline void __bit_spin_unlock(int bitnum, unsigned long *addr)
 	__release(bitlock);
 }
 
+/**
+ * bit_spin_unlock_assign - Unlock a bitlock by assignment of new value.
+ * @addr: Address to assign the value to.
+ * @val: New value to assign.
+ * @lock: Split lock that this bitlock is part of.
+ */
+static inline void bit_spin_unlock_assign(unsigned long *addr,
+		unsigned long val, struct split_lock *lock)
+{
+	smp_store_release(addr, val);
+	preempt_enable();
+	__release(bitlock);
+}
+
 /*
  * Return true if the lock is held.
  */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 04/17] hlist_bl: Prepare for split_locks
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (2 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 03/17] bit_spinlock: Prepare for split_locks Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 05/17] dm-snap: Add dm_exceptional_lock Matthew Wilcox (Oracle)
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Make hlist_bl_lock() and hlist_bl_unlock() variadic to help with the
transition.  Also add hlist_bl_lock_nested().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/list_bl.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h
index ae1b541446c9..1bfdb441c8bc 100644
--- a/include/linux/list_bl.h
+++ b/include/linux/list_bl.h
@@ -143,12 +143,19 @@ static inline void hlist_bl_del_init(struct hlist_bl_node *n)
 	}
 }
 
-static inline void hlist_bl_lock(struct hlist_bl_head *b)
+static inline void hlist_bl_lock(struct hlist_bl_head *b, ...)
 {
 	bit_spin_lock(0, (unsigned long *)b);
 }
 
-static inline void hlist_bl_unlock(struct hlist_bl_head *b)
+static inline void hlist_bl_lock_nested(struct hlist_bl_head *b,
+		struct split_lock *sl, unsigned int subclass)
+{
+	bit_spin_lock_nested(0, (unsigned long *)b, sl, subclass);
+}
+
+static inline void hlist_bl_unlock(struct hlist_bl_head *b,
+					...)
 {
 	__bit_spin_unlock(0, (unsigned long *)b);
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 05/17] dm-snap: Add dm_exceptional_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (3 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 04/17] hlist_bl: " Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 06/17] dcache: Add d_hash_lock Matthew Wilcox (Oracle)
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the dm-snap bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 drivers/md/dm-snap.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 8f3ad87e6117..4c2a01e433de 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -644,16 +644,18 @@ static void dm_exception_table_lock_init(struct dm_snapshot *s, chunk_t chunk,
 	lock->pending_slot = &pending->table[exception_hash(pending, chunk)];
 }
 
+static DEFINE_SPLIT_LOCK(dm_exception_lock);
+
 static void dm_exception_table_lock(struct dm_exception_table_lock *lock)
 {
-	hlist_bl_lock(lock->complete_slot);
-	hlist_bl_lock(lock->pending_slot);
+	hlist_bl_lock(lock->complete_slot, &dm_exception_lock);
+	hlist_bl_lock_nested(lock->pending_slot, &dm_exception_lock, 1);
 }
 
 static void dm_exception_table_unlock(struct dm_exception_table_lock *lock)
 {
-	hlist_bl_unlock(lock->pending_slot);
-	hlist_bl_unlock(lock->complete_slot);
+	hlist_bl_unlock(lock->pending_slot, &dm_exception_lock);
+	hlist_bl_unlock(lock->complete_slot, &dm_exception_lock);
 }
 
 static int dm_exception_table_init(struct dm_exception_table *et,
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 06/17] dcache: Add d_hash_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (4 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 05/17] dm-snap: Add dm_exceptional_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 07/17] fscache: Add cookie_hash_lock Matthew Wilcox (Oracle)
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the d_hash bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/dcache.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 7d24ff7eb206..a3861d330001 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -96,6 +96,7 @@ EXPORT_SYMBOL(slash_name);
 
 static unsigned int d_hash_shift __read_mostly;
 
+static DEFINE_SPLIT_LOCK(d_hash_lock);
 static struct hlist_bl_head *dentry_hashtable __read_mostly;
 
 static inline struct hlist_bl_head *d_hash(unsigned int hash)
@@ -469,9 +470,9 @@ static void ___d_drop(struct dentry *dentry)
 	else
 		b = d_hash(dentry->d_name.hash);
 
-	hlist_bl_lock(b);
+	hlist_bl_lock(b, &d_hash_lock);
 	__hlist_bl_del(&dentry->d_hash);
-	hlist_bl_unlock(b);
+	hlist_bl_unlock(b, &d_hash_lock);
 }
 
 void __d_drop(struct dentry *dentry)
@@ -2074,9 +2075,9 @@ static struct dentry *__d_instantiate_anon(struct dentry *dentry,
 	__d_set_inode_and_type(dentry, inode, add_flags);
 	hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry);
 	if (!disconnected) {
-		hlist_bl_lock(&dentry->d_sb->s_roots);
+		hlist_bl_lock(&dentry->d_sb->s_roots, &d_hash_lock);
 		hlist_bl_add_head(&dentry->d_hash, &dentry->d_sb->s_roots);
-		hlist_bl_unlock(&dentry->d_sb->s_roots);
+		hlist_bl_unlock(&dentry->d_sb->s_roots, &d_hash_lock);
 	}
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&inode->i_lock);
@@ -2513,9 +2514,9 @@ static void __d_rehash(struct dentry *entry)
 {
 	struct hlist_bl_head *b = d_hash(entry->d_name.hash);
 
-	hlist_bl_lock(b);
+	hlist_bl_lock(b, &d_hash_lock);
 	hlist_bl_add_head_rcu(&entry->d_hash, b);
-	hlist_bl_unlock(b);
+	hlist_bl_unlock(b, &d_hash_lock);
 }
 
 /**
@@ -2606,9 +2607,9 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 		goto retry;
 	}
 
-	hlist_bl_lock(b);
+	hlist_bl_lock(b, &d_hash_lock);
 	if (unlikely(READ_ONCE(parent->d_inode->i_dir_seq) != seq)) {
-		hlist_bl_unlock(b);
+		hlist_bl_unlock(b, &d_hash_lock);
 		rcu_read_unlock();
 		goto retry;
 	}
@@ -2626,7 +2627,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 			continue;
 		if (!d_same_name(dentry, parent, name))
 			continue;
-		hlist_bl_unlock(b);
+		hlist_bl_unlock(b, &d_hash_lock);
 		/* now we can try to grab a reference */
 		if (!lockref_get_not_dead(&dentry->d_lockref)) {
 			rcu_read_unlock();
@@ -2664,7 +2665,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 	new->d_flags |= DCACHE_PAR_LOOKUP;
 	new->d_wait = wq;
 	hlist_bl_add_head_rcu(&new->d_u.d_in_lookup_hash, b);
-	hlist_bl_unlock(b);
+	hlist_bl_unlock(b, &d_hash_lock);
 	return new;
 mismatch:
 	spin_unlock(&dentry->d_lock);
@@ -2677,12 +2678,12 @@ void __d_lookup_done(struct dentry *dentry)
 {
 	struct hlist_bl_head *b = in_lookup_hash(dentry->d_parent,
 						 dentry->d_name.hash);
-	hlist_bl_lock(b);
+	hlist_bl_lock(b, &d_hash_lock);
 	dentry->d_flags &= ~DCACHE_PAR_LOOKUP;
 	__hlist_bl_del(&dentry->d_u.d_in_lookup_hash);
 	wake_up_all(dentry->d_wait);
 	dentry->d_wait = NULL;
-	hlist_bl_unlock(b);
+	hlist_bl_unlock(b, &d_hash_lock);
 	INIT_HLIST_NODE(&dentry->d_u.d_alias);
 	INIT_LIST_HEAD(&dentry->d_lru);
 }
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 07/17] fscache: Add cookie_hash_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (5 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 06/17] dcache: Add d_hash_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 08/17] gfs2: Add qd_hash_lock Matthew Wilcox (Oracle)
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the fscache cookie hash bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/fscache/cookie.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index 751bc5b1cddf..65d514d12592 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -17,6 +17,7 @@ struct kmem_cache *fscache_cookie_jar;
 
 static atomic_t fscache_object_debug_id = ATOMIC_INIT(0);
 
+static DEFINE_SPLIT_LOCK(cookie_hash_lock);
 #define fscache_cookie_hash_shift 15
 static struct hlist_bl_head fscache_cookie_hash[1 << fscache_cookie_hash_shift];
 
@@ -202,7 +203,7 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)
 	bucket = candidate->key_hash & (ARRAY_SIZE(fscache_cookie_hash) - 1);
 	h = &fscache_cookie_hash[bucket];
 
-	hlist_bl_lock(h);
+	hlist_bl_lock(h, &cookie_hash_lock);
 	hlist_bl_for_each_entry(cursor, p, h, hash_link) {
 		if (fscache_compare_cookie(candidate, cursor) == 0)
 			goto collision;
@@ -212,7 +213,7 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)
 	fscache_cookie_get(candidate->parent, fscache_cookie_get_acquire_parent);
 	atomic_inc(&candidate->parent->n_children);
 	hlist_bl_add_head(&candidate->hash_link, h);
-	hlist_bl_unlock(h);
+	hlist_bl_unlock(h, &cookie_hash_lock);
 	return candidate;
 
 collision:
@@ -222,12 +223,12 @@ struct fscache_cookie *fscache_hash_cookie(struct fscache_cookie *candidate)
 		pr_err("Duplicate cookie detected\n");
 		fscache_print_cookie(cursor, 'O');
 		fscache_print_cookie(candidate, 'N');
-		hlist_bl_unlock(h);
+		hlist_bl_unlock(h, &cookie_hash_lock);
 		return NULL;
 	}
 
 	fscache_cookie_get(cursor, fscache_cookie_get_reacquire);
-	hlist_bl_unlock(h);
+	hlist_bl_unlock(h, &cookie_hash_lock);
 	return cursor;
 }
 
@@ -845,9 +846,9 @@ static void fscache_unhash_cookie(struct fscache_cookie *cookie)
 	bucket = cookie->key_hash & (ARRAY_SIZE(fscache_cookie_hash) - 1);
 	h = &fscache_cookie_hash[bucket];
 
-	hlist_bl_lock(h);
+	hlist_bl_lock(h, &cookie_hash_lock);
 	hlist_bl_del(&cookie->hash_link);
-	hlist_bl_unlock(h);
+	hlist_bl_unlock(h, &cookie_hash_lock);
 }
 
 /*
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 08/17] gfs2: Add qd_hash_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (6 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 07/17] fscache: Add cookie_hash_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 09/17] mbcache: Add mb_cache_lock Matthew Wilcox (Oracle)
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the hash bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/quota.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 9b1aca7e1264..a933eb441ee9 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -80,6 +80,7 @@
 static DEFINE_SPINLOCK(qd_lock);
 struct list_lru gfs2_qd_lru;
 
+static DEFINE_SPLIT_LOCK(qd_hash_lock);
 static struct hlist_bl_head qd_hash_table[GFS2_QD_HASH_SIZE];
 
 static unsigned int gfs2_qd_hash(const struct gfs2_sbd *sdp,
@@ -95,12 +96,12 @@ static unsigned int gfs2_qd_hash(const struct gfs2_sbd *sdp,
 
 static inline void spin_lock_bucket(unsigned int hash)
 {
-        hlist_bl_lock(&qd_hash_table[hash]);
+        hlist_bl_lock(&qd_hash_table[hash], &qd_hash_lock);
 }
 
 static inline void spin_unlock_bucket(unsigned int hash)
 {
-        hlist_bl_unlock(&qd_hash_table[hash]);
+        hlist_bl_unlock(&qd_hash_table[hash], &qd_hash_lock);
 }
 
 static void gfs2_qd_dealloc(struct rcu_head *rcu)
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 09/17] mbcache: Add mb_cache_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (7 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 08/17] gfs2: Add qd_hash_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 10/17] hlist_bl: Make the split_lock parameter mandatory Matthew Wilcox (Oracle)
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the mbcache hash bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/mbcache.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/mbcache.c b/fs/mbcache.c
index 97c54d3a2227..4ce03ea348dd 100644
--- a/fs/mbcache.c
+++ b/fs/mbcache.c
@@ -47,6 +47,7 @@ static struct kmem_cache *mb_entry_cache;
 static unsigned long mb_cache_shrink(struct mb_cache *cache,
 				     unsigned long nr_to_scan);
 
+static DEFINE_SPLIT_LOCK(mb_cache_lock);
 static inline struct hlist_bl_head *mb_cache_entry_head(struct mb_cache *cache,
 							u32 key)
 {
@@ -97,16 +98,16 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
 	entry->e_reusable = reusable;
 	entry->e_referenced = 0;
 	head = mb_cache_entry_head(cache, key);
-	hlist_bl_lock(head);
+	hlist_bl_lock(head, &mb_cache_lock);
 	hlist_bl_for_each_entry(dup, dup_node, head, e_hash_list) {
 		if (dup->e_key == key && dup->e_value == value) {
-			hlist_bl_unlock(head);
+			hlist_bl_unlock(head, &mb_cache_lock);
 			kmem_cache_free(mb_entry_cache, entry);
 			return -EBUSY;
 		}
 	}
 	hlist_bl_add_head(&entry->e_hash_list, head);
-	hlist_bl_unlock(head);
+	hlist_bl_unlock(head, &mb_cache_lock);
 
 	spin_lock(&cache->c_list_lock);
 	list_add_tail(&entry->e_list, &cache->c_list);
@@ -134,7 +135,7 @@ static struct mb_cache_entry *__entry_find(struct mb_cache *cache,
 	struct hlist_bl_head *head;
 
 	head = mb_cache_entry_head(cache, key);
-	hlist_bl_lock(head);
+	hlist_bl_lock(head, &mb_cache_lock);
 	if (entry && !hlist_bl_unhashed(&entry->e_hash_list))
 		node = entry->e_hash_list.next;
 	else
@@ -150,7 +151,7 @@ static struct mb_cache_entry *__entry_find(struct mb_cache *cache,
 	}
 	entry = NULL;
 out:
-	hlist_bl_unlock(head);
+	hlist_bl_unlock(head, &mb_cache_lock);
 	if (old_entry)
 		mb_cache_entry_put(cache, old_entry);
 
@@ -203,7 +204,7 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
 	struct mb_cache_entry *entry;
 
 	head = mb_cache_entry_head(cache, key);
-	hlist_bl_lock(head);
+	hlist_bl_lock(head, &mb_cache_lock);
 	hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
 		if (entry->e_key == key && entry->e_value == value) {
 			atomic_inc(&entry->e_refcnt);
@@ -212,7 +213,7 @@ struct mb_cache_entry *mb_cache_entry_get(struct mb_cache *cache, u32 key,
 	}
 	entry = NULL;
 out:
-	hlist_bl_unlock(head);
+	hlist_bl_unlock(head, &mb_cache_lock);
 	return entry;
 }
 EXPORT_SYMBOL(mb_cache_entry_get);
@@ -231,12 +232,12 @@ void mb_cache_entry_delete(struct mb_cache *cache, u32 key, u64 value)
 	struct mb_cache_entry *entry;
 
 	head = mb_cache_entry_head(cache, key);
-	hlist_bl_lock(head);
+	hlist_bl_lock(head, &mb_cache_lock);
 	hlist_bl_for_each_entry(entry, node, head, e_hash_list) {
 		if (entry->e_key == key && entry->e_value == value) {
 			/* We keep hash list reference to keep entry alive */
 			hlist_bl_del_init(&entry->e_hash_list);
-			hlist_bl_unlock(head);
+			hlist_bl_unlock(head, &mb_cache_lock);
 			spin_lock(&cache->c_list_lock);
 			if (!list_empty(&entry->e_list)) {
 				list_del_init(&entry->e_list);
@@ -250,7 +251,7 @@ void mb_cache_entry_delete(struct mb_cache *cache, u32 key, u64 value)
 			return;
 		}
 	}
-	hlist_bl_unlock(head);
+	hlist_bl_unlock(head, &mb_cache_lock);
 }
 EXPORT_SYMBOL(mb_cache_entry_delete);
 
@@ -301,12 +302,12 @@ static unsigned long mb_cache_shrink(struct mb_cache *cache,
 		 */
 		spin_unlock(&cache->c_list_lock);
 		head = mb_cache_entry_head(cache, entry->e_key);
-		hlist_bl_lock(head);
+		hlist_bl_lock(head, &mb_cache_lock);
 		if (!hlist_bl_unhashed(&entry->e_hash_list)) {
 			hlist_bl_del_init(&entry->e_hash_list);
 			atomic_dec(&entry->e_refcnt);
 		}
-		hlist_bl_unlock(head);
+		hlist_bl_unlock(head, &mb_cache_lock);
 		if (mb_cache_entry_put(cache, entry))
 			shrunk++;
 		cond_resched();
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 10/17] hlist_bl: Make the split_lock parameter mandatory
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (8 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 09/17] mbcache: Add mb_cache_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 11/17] s390: Add airq_iv_lock Matthew Wilcox (Oracle)
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Now that all users have been converted, require the split_lock parameter
be passed to hlist_bl_lock() and hlist_bl_unlock().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/list_bl.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/list_bl.h b/include/linux/list_bl.h
index 1bfdb441c8bc..eb1faa4a4e8c 100644
--- a/include/linux/list_bl.h
+++ b/include/linux/list_bl.h
@@ -143,9 +143,9 @@ static inline void hlist_bl_del_init(struct hlist_bl_node *n)
 	}
 }
 
-static inline void hlist_bl_lock(struct hlist_bl_head *b, ...)
+static inline void hlist_bl_lock(struct hlist_bl_head *b, struct split_lock *sl)
 {
-	bit_spin_lock(0, (unsigned long *)b);
+	bit_spin_lock(0, (unsigned long *)b, sl);
 }
 
 static inline void hlist_bl_lock_nested(struct hlist_bl_head *b,
@@ -155,9 +155,9 @@ static inline void hlist_bl_lock_nested(struct hlist_bl_head *b,
 }
 
 static inline void hlist_bl_unlock(struct hlist_bl_head *b,
-					...)
+					struct split_lock *sl)
 {
-	__bit_spin_unlock(0, (unsigned long *)b);
+	__bit_spin_unlock(0, (unsigned long *)b, sl);
 }
 
 static inline bool hlist_bl_is_locked(struct hlist_bl_head *b)
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 11/17] s390: Add airq_iv_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (9 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 10/17] hlist_bl: Make the split_lock parameter mandatory Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  6:18   ` kernel test robot
  2021-04-09  2:51 ` [PATCH 12/17] zram: Add zram_table_lock Matthew Wilcox (Oracle)
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the airq bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 arch/s390/include/asm/airq.h | 5 +++--
 drivers/s390/cio/airq.c      | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index 01936fdfaddb..d281340de14a 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -26,6 +26,7 @@ struct airq_struct {
 
 int register_adapter_interrupt(struct airq_struct *airq);
 void unregister_adapter_interrupt(struct airq_struct *airq);
+extern struct iv_lock airq_iv_lock;
 
 /* Adapter interrupt bit vector */
 struct airq_iv {
@@ -72,13 +73,13 @@ static inline unsigned long airq_iv_end(struct airq_iv *iv)
 static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
 {
 	const unsigned long be_to_le = BITS_PER_LONG - 1;
-	bit_spin_lock(bit ^ be_to_le, iv->bitlock);
+	bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
 }
 
 static inline void airq_iv_unlock(struct airq_iv *iv, unsigned long bit)
 {
 	const unsigned long be_to_le = BITS_PER_LONG - 1;
-	bit_spin_unlock(bit ^ be_to_le, iv->bitlock);
+	bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
 }
 
 static inline void airq_iv_set_data(struct airq_iv *iv, unsigned long bit,
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index cb466ed7eb5e..6e850661957c 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -31,6 +31,9 @@ static struct hlist_head airq_lists[MAX_ISC+1];
 
 static struct dma_pool *airq_iv_cache;
 
+DEFINE_SPLIT_LOCK(airq_iv_lock);
+EXPORT_SYMBOL(airq_iv_lock);
+
 /**
  * register_adapter_interrupt() - register adapter interrupt handler
  * @airq: pointer to adapter interrupt descriptor
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 12/17] zram: Add zram_table_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (10 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 11/17] s390: Add airq_iv_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 13/17] jbd2: Add jbd2_jh_lock Matthew Wilcox (Oracle)
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the zram bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 drivers/block/zram/zram_drv.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index cf8deecc39ef..8b678cc6ed21 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -59,20 +59,22 @@ static void zram_free_page(struct zram *zram, size_t index);
 static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec,
 				u32 index, int offset, struct bio *bio);
 
+static DEFINE_SPLIT_LOCK(zram_table_lock);
 
 static int zram_slot_trylock(struct zram *zram, u32 index)
 {
-	return bit_spin_trylock(ZRAM_LOCK, &zram->table[index].flags);
+	return bit_spin_trylock(ZRAM_LOCK, &zram->table[index].flags,
+			&zram_table_lock);
 }
 
 static void zram_slot_lock(struct zram *zram, u32 index)
 {
-	bit_spin_lock(ZRAM_LOCK, &zram->table[index].flags);
+	bit_spin_lock(ZRAM_LOCK, &zram->table[index].flags, &zram_table_lock);
 }
 
 static void zram_slot_unlock(struct zram *zram, u32 index)
 {
-	bit_spin_unlock(ZRAM_LOCK, &zram->table[index].flags);
+	bit_spin_unlock(ZRAM_LOCK, &zram->table[index].flags, &zram_table_lock);
 }
 
 static inline bool init_done(struct zram *zram)
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 13/17] jbd2: Add jbd2_jh_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (11 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 12/17] zram: Add zram_table_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 14/17] slub: Add slab_page_lock Matthew Wilcox (Oracle)
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track the journal bit spin locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/jbd2/journal.c    | 18 ++++++++++--------
 include/linux/jbd2.h | 10 ++++++----
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 2dc944442802..9c5f4b99157b 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2733,6 +2733,8 @@ static void journal_free_journal_head(struct journal_head *jh)
  *	jbd2_journal_put_journal_head(jh);
  */
 
+static DEFINE_SPLIT_LOCK(jbd2_jh_lock);
+
 /*
  * Give a buffer_head a journal_head.
  *
@@ -2747,7 +2749,7 @@ struct journal_head *jbd2_journal_add_journal_head(struct buffer_head *bh)
 	if (!buffer_jbd(bh))
 		new_jh = journal_alloc_journal_head();
 
-	jbd_lock_bh_journal_head(bh);
+	jbd_lock_bh_journal_head(bh, &jbd2_jh_lock);
 	if (buffer_jbd(bh)) {
 		jh = bh2jh(bh);
 	} else {
@@ -2756,7 +2758,7 @@ struct journal_head *jbd2_journal_add_journal_head(struct buffer_head *bh)
 			(bh->b_page && bh->b_page->mapping));
 
 		if (!new_jh) {
-			jbd_unlock_bh_journal_head(bh);
+			jbd_unlock_bh_journal_head(bh, &jbd2_jh_lock);
 			goto repeat;
 		}
 
@@ -2769,7 +2771,7 @@ struct journal_head *jbd2_journal_add_journal_head(struct buffer_head *bh)
 		BUFFER_TRACE(bh, "added journal_head");
 	}
 	jh->b_jcount++;
-	jbd_unlock_bh_journal_head(bh);
+	jbd_unlock_bh_journal_head(bh, &jbd2_jh_lock);
 	if (new_jh)
 		journal_free_journal_head(new_jh);
 	return bh->b_private;
@@ -2783,12 +2785,12 @@ struct journal_head *jbd2_journal_grab_journal_head(struct buffer_head *bh)
 {
 	struct journal_head *jh = NULL;
 
-	jbd_lock_bh_journal_head(bh);
+	jbd_lock_bh_journal_head(bh, &jbd2_jh_lock);
 	if (buffer_jbd(bh)) {
 		jh = bh2jh(bh);
 		jh->b_jcount++;
 	}
-	jbd_unlock_bh_journal_head(bh);
+	jbd_unlock_bh_journal_head(bh, &jbd2_jh_lock);
 	return jh;
 }
 
@@ -2831,16 +2833,16 @@ void jbd2_journal_put_journal_head(struct journal_head *jh)
 {
 	struct buffer_head *bh = jh2bh(jh);
 
-	jbd_lock_bh_journal_head(bh);
+	jbd_lock_bh_journal_head(bh, &jbd2_jh_lock);
 	J_ASSERT_JH(jh, jh->b_jcount > 0);
 	--jh->b_jcount;
 	if (!jh->b_jcount) {
 		__journal_remove_journal_head(bh);
-		jbd_unlock_bh_journal_head(bh);
+		jbd_unlock_bh_journal_head(bh, &jbd2_jh_lock);
 		journal_release_journal_head(jh, bh->b_size);
 		__brelse(bh);
 	} else {
-		jbd_unlock_bh_journal_head(bh);
+		jbd_unlock_bh_journal_head(bh, &jbd2_jh_lock);
 	}
 }
 
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 99d3cd051ac3..bf32cee8b1e2 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -348,14 +348,16 @@ static inline struct journal_head *bh2jh(struct buffer_head *bh)
 	return bh->b_private;
 }
 
-static inline void jbd_lock_bh_journal_head(struct buffer_head *bh)
+static inline void jbd_lock_bh_journal_head(struct buffer_head *bh,
+		struct split_lock *sl)
 {
-	bit_spin_lock(BH_JournalHead, &bh->b_state);
+	bit_spin_lock(BH_JournalHead, &bh->b_state, sl);
 }
 
-static inline void jbd_unlock_bh_journal_head(struct buffer_head *bh)
+static inline void jbd_unlock_bh_journal_head(struct buffer_head *bh,
+		struct split_lock *sl)
 {
-	bit_spin_unlock(BH_JournalHead, &bh->b_state);
+	bit_spin_unlock(BH_JournalHead, &bh->b_state, sl);
 }
 
 #define J_ASSERT(assert)	BUG_ON(!(assert))
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 14/17] slub: Add slab_page_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (12 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 13/17] jbd2: Add jbd2_jh_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 15/17] zsmalloc: Add zs_pin_lock Matthew Wilcox (Oracle)
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track slub's page bit spin lock.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/slub.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9c0e26ddf300..2ed2abe080ac 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -346,19 +346,21 @@ static inline unsigned int oo_objects(struct kmem_cache_order_objects x)
 	return x.x & OO_MASK;
 }
 
+static DEFINE_SPLIT_LOCK(slab_page_lock);
+
 /*
  * Per slab locking using the pagelock
  */
 static __always_inline void slab_lock(struct page *page)
 {
 	VM_BUG_ON_PAGE(PageTail(page), page);
-	bit_spin_lock(PG_locked, &page->flags);
+	bit_spin_lock(PG_locked, &page->flags, &slab_page_lock);
 }
 
 static __always_inline void slab_unlock(struct page *page)
 {
 	VM_BUG_ON_PAGE(PageTail(page), page);
-	__bit_spin_unlock(PG_locked, &page->flags);
+	__bit_spin_unlock(PG_locked, &page->flags, &slab_page_lock);
 }
 
 /* Interrupts must be disabled (for the fallback code to work right) */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 15/17] zsmalloc: Add zs_pin_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (13 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 14/17] slub: Add slab_page_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 16/17] rhashtable: Convert to split_lock Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep Matthew Wilcox (Oracle)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Allow lockdep to track zsmalloc's pin bit spin lock.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/zsmalloc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 9a7c91c14b84..9d89a1857901 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -874,6 +874,8 @@ static unsigned long obj_to_head(struct page *page, void *obj)
 		return *(unsigned long *)obj;
 }
 
+static DEFINE_SPLIT_LOCK(zs_pin_lock);
+
 static inline int testpin_tag(unsigned long handle)
 {
 	return bit_spin_is_locked(HANDLE_PIN_BIT, (unsigned long *)handle);
@@ -881,17 +883,20 @@ static inline int testpin_tag(unsigned long handle)
 
 static inline int trypin_tag(unsigned long handle)
 {
-	return bit_spin_trylock(HANDLE_PIN_BIT, (unsigned long *)handle);
+	return bit_spin_trylock(HANDLE_PIN_BIT, (unsigned long *)handle,
+				&zs_pin_lock);
 }
 
 static void pin_tag(unsigned long handle) __acquires(bitlock)
 {
-	bit_spin_lock(HANDLE_PIN_BIT, (unsigned long *)handle);
+	bit_spin_lock(HANDLE_PIN_BIT, (unsigned long *)handle,
+			&zs_pin_lock);
 }
 
 static void unpin_tag(unsigned long handle) __releases(bitlock)
 {
-	bit_spin_unlock(HANDLE_PIN_BIT, (unsigned long *)handle);
+	bit_spin_unlock(HANDLE_PIN_BIT, (unsigned long *)handle,
+			&zs_pin_lock);
 }
 
 static void reset_page(struct page *page)
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 16/17] rhashtable: Convert to split_lock
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (14 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 15/17] zsmalloc: Add zs_pin_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  2:51 ` [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep Matthew Wilcox (Oracle)
  16 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

NeilBrown noticed the same problem with bit spinlocks that I did,
but chose to solve it locally in the rhashtable implementation rather
than lift it all the way to the bit spin lock implementation.  Convert
rhashtables to use split_locks.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: NeilBrown <neilb@suse.de>
---
 include/linux/rhashtable.h | 20 +++++++-------------
 lib/rhashtable.c           |  5 +----
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 68dab3e08aad..4df164fe6222 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -65,12 +65,11 @@ struct rhash_lock_head {};
  * struct bucket_table - Table of hash buckets
  * @size: Number of hash buckets
  * @nest: Number of bits of first-level nested table.
- * @rehash: Current bucket being rehashed
  * @hash_rnd: Random seed to fold into hash
  * @walkers: List of active walkers
  * @rcu: RCU structure for freeing the table
  * @future_tbl: Table under construction during rehashing
- * @ntbl: Nested table used when out of memory.
+ * @sl: Conceptual spinlock representing every per-bucket lock.
  * @buckets: size * hash buckets
  */
 struct bucket_table {
@@ -82,7 +81,7 @@ struct bucket_table {
 
 	struct bucket_table __rcu *future_tbl;
 
-	struct lockdep_map	dep_map;
+	struct split_lock	sl;
 
 	struct rhash_lock_head __rcu *buckets[] ____cacheline_aligned_in_smp;
 };
@@ -327,8 +326,7 @@ static inline void rht_lock(struct bucket_table *tbl,
 			    struct rhash_lock_head __rcu **bkt)
 {
 	local_bh_disable();
-	bit_spin_lock(0, (unsigned long *)bkt);
-	lock_map_acquire(&tbl->dep_map);
+	bit_spin_lock(0, (unsigned long *)bkt, &tbl->sl);
 }
 
 static inline void rht_lock_nested(struct bucket_table *tbl,
@@ -336,15 +334,13 @@ static inline void rht_lock_nested(struct bucket_table *tbl,
 				   unsigned int subclass)
 {
 	local_bh_disable();
-	bit_spin_lock(0, (unsigned long *)bucket);
-	lock_acquire_exclusive(&tbl->dep_map, subclass, 0, NULL, _THIS_IP_);
+	bit_spin_lock_nested(0, (unsigned long *)bucket, &tbl->sl, subclass);
 }
 
 static inline void rht_unlock(struct bucket_table *tbl,
 			      struct rhash_lock_head __rcu **bkt)
 {
-	lock_map_release(&tbl->dep_map);
-	bit_spin_unlock(0, (unsigned long *)bkt);
+	bit_spin_unlock(0, (unsigned long *)bkt, &tbl->sl);
 	local_bh_enable();
 }
 
@@ -397,10 +393,8 @@ static inline void rht_assign_unlock(struct bucket_table *tbl,
 {
 	if (rht_is_a_nulls(obj))
 		obj = NULL;
-	lock_map_release(&tbl->dep_map);
-	rcu_assign_pointer(*bkt, (void *)obj);
-	preempt_enable();
-	__release(bitlock);
+	bit_spin_unlock_assign((unsigned long *)bkt, (unsigned long)obj,
+				&tbl->sl);
 	local_bh_enable();
 }
 
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index c949c1e3b87c..bfdb0bf87f99 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -179,7 +179,6 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	struct bucket_table *tbl = NULL;
 	size_t size;
 	int i;
-	static struct lock_class_key __key;
 
 	tbl = kvzalloc(struct_size(tbl, buckets, nbuckets), gfp);
 
@@ -193,10 +192,8 @@ static struct bucket_table *bucket_table_alloc(struct rhashtable *ht,
 	if (tbl == NULL)
 		return NULL;
 
-	lockdep_init_map(&tbl->dep_map, "rhashtable_bucket", &__key, 0);
-
+	split_lock_init(&tbl->sl);
 	tbl->size = size;
-
 	rcu_head_init(&tbl->rcu);
 	INIT_LIST_HEAD(&tbl->walkers);
 
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep
  2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
                   ` (15 preceding siblings ...)
  2021-04-09  2:51 ` [PATCH 16/17] rhashtable: Convert to split_lock Matthew Wilcox (Oracle)
@ 2021-04-09  2:51 ` Matthew Wilcox (Oracle)
  2021-04-09  6:37   ` kernel test robot
  16 siblings, 1 reply; 28+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-04-09  2:51 UTC (permalink / raw)
  To: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: Matthew Wilcox (Oracle), linux-kernel

Now that all users have been converted, require the split_lock parameter
be passed to bit_spin_lock(), bit_spin_unlock() and variants.  Use it
to track the lockdep state of each lock.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/bit_spinlock.h | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/include/linux/bit_spinlock.h b/include/linux/bit_spinlock.h
index 6c5bbb55b334..a02ce695198a 100644
--- a/include/linux/bit_spinlock.h
+++ b/include/linux/bit_spinlock.h
@@ -34,30 +34,21 @@ static inline void bit_spin_lock_nested(int bitnum, unsigned long *addr,
 		preempt_disable();
 	}
 #endif
+	spin_acquire(&lock->dep_map, subclass, 0, _RET_IP_);
 	__acquire(bitlock);
 }
 
 static inline void bit_spin_lock(int bitnum, unsigned long *addr,
-		...)
+		struct split_lock *lock)
 {
-	preempt_disable();
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-	while (unlikely(test_and_set_bit_lock(bitnum, addr))) {
-		preempt_enable();
-		do {
-			cpu_relax();
-		} while (test_bit(bitnum, addr));
-		preempt_disable();
-	}
-#endif
-	__acquire(bitlock);
+	bit_spin_lock_nested(bitnum, addr, lock, 0);
 }
 
 /*
  * Return true if it was acquired
  */
 static inline int bit_spin_trylock(int bitnum, unsigned long *addr,
-		...)
+		struct split_lock *lock)
 {
 	preempt_disable();
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
@@ -66,6 +57,7 @@ static inline int bit_spin_trylock(int bitnum, unsigned long *addr,
 		return 0;
 	}
 #endif
+	spin_acquire(&lock->dep_map, 0, 1, _RET_IP_);
 	__acquire(bitlock);
 	return 1;
 }
@@ -74,11 +66,12 @@ static inline int bit_spin_trylock(int bitnum, unsigned long *addr,
  *  bit-based spin_unlock()
  */
 static inline void bit_spin_unlock(int bitnum, unsigned long *addr,
-		...)
+		struct split_lock *lock)
 {
 #ifdef CONFIG_DEBUG_SPINLOCK
 	BUG_ON(!test_bit(bitnum, addr));
 #endif
+	spin_release(&lock->dep_map, _RET_IP_);
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
 	clear_bit_unlock(bitnum, addr);
 #endif
@@ -92,11 +85,12 @@ static inline void bit_spin_unlock(int bitnum, unsigned long *addr,
  *  protecting the rest of the flags in the word.
  */
 static inline void __bit_spin_unlock(int bitnum, unsigned long *addr,
-		...)
+		struct split_lock *lock)
 {
 #ifdef CONFIG_DEBUG_SPINLOCK
 	BUG_ON(!test_bit(bitnum, addr));
 #endif
+	spin_release(&lock->dep_map, _RET_IP_);
 #if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
 	__clear_bit_unlock(bitnum, addr);
 #endif
@@ -113,6 +107,7 @@ static inline void __bit_spin_unlock(int bitnum, unsigned long *addr,
 static inline void bit_spin_unlock_assign(unsigned long *addr,
 		unsigned long val, struct split_lock *lock)
 {
+	spin_release(&lock->dep_map, _RET_IP_);
 	smp_store_release(addr, val);
 	preempt_enable();
 	__release(bitlock);
@@ -133,4 +128,3 @@ static inline int bit_spin_is_locked(int bitnum, unsigned long *addr)
 }
 
 #endif /* __LINUX_BIT_SPINLOCK_H */
-
-- 
2.30.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 11/17] s390: Add airq_iv_lock
  2021-04-09  2:51 ` [PATCH 11/17] s390: Add airq_iv_lock Matthew Wilcox (Oracle)
@ 2021-04-09  6:18   ` kernel test robot
  2021-04-09 13:20     ` Matthew Wilcox
  0 siblings, 1 reply; 28+ messages in thread
From: kernel test robot @ 2021-04-09  6:18 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle),
	neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: kbuild-all, Matthew Wilcox (Oracle), linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5618 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210408]
[cannot apply to s390/features tip/x86/core dm/for-next gfs2/for-next block/for-next linus/master hnaz-linux-mm/master v5.12-rc6 v5.12-rc5 v5.12-rc4 v5.12-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Provide-lockdep-tracking-for-bit-spin-locks/20210409-110522
base:    6145d80cfc62e3ed8f16ff584d6287e6d88b82b9
config: s390-randconfig-r012-20210409 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6592d0d634dacfb6fd14d9a659050a85e39a953e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Provide-lockdep-tracking-for-bit-spin-locks/20210409-110522
        git checkout 6592d0d634dacfb6fd14d9a659050a85e39a953e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/s390/pci/pci.c:33:
>> arch/s390/include/asm/airq.h:73:20: error: 'airq_iv_lock' redeclared as different kind of symbol
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~
--
   In file included from drivers/s390/cio/airq.c:21:
>> arch/s390/include/asm/airq.h:73:20: error: 'airq_iv_lock' redeclared as different kind of symbol
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~
   In file included from include/linux/bit_spinlock.h:9,
                    from include/linux/mm.h:22,
                    from include/linux/scatterlist.h:8,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1464,
                    from arch/s390/include/asm/hw_irq.h:6,
                    from include/linux/irq.h:589,
                    from drivers/s390/cio/airq.c:13:
>> drivers/s390/cio/airq.c:34:19: error: 'airq_iv_lock' redeclared as different kind of symbol
      34 | DEFINE_SPLIT_LOCK(airq_iv_lock);
         |                   ^~~~~~~~~~~~
   include/linux/split_lock.h:23:19: note: in definition of macro 'DEFINE_SPLIT_LOCK'
      23 | struct split_lock name = {      \
         |                   ^~~~
   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h:73:20: note: previous definition of 'airq_iv_lock' was here
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:51,
                    from include/linux/irq.h:14,
                    from drivers/s390/cio/airq.c:13:
>> drivers/s390/cio/airq.c:35:15: error: conflicting types for 'airq_iv_lock'
      35 | EXPORT_SYMBOL(airq_iv_lock);
         |               ^~~~~~~~~~~~
   include/linux/export.h:98:21: note: in definition of macro '___EXPORT_SYMBOL'
      98 |  extern typeof(sym) sym;       \
         |                     ^~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:158:29: note: in expansion of macro '_EXPORT_SYMBOL'
     158 | #define EXPORT_SYMBOL(sym)  _EXPORT_SYMBOL(sym, "")
         |                             ^~~~~~~~~~~~~~
   drivers/s390/cio/airq.c:35:1: note: in expansion of macro 'EXPORT_SYMBOL'
      35 | EXPORT_SYMBOL(airq_iv_lock);
         | ^~~~~~~~~~~~~
   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~


vim +/airq_iv_lock +73 arch/s390/include/asm/airq.h

a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  72  
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25 @73  static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  74  {
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  75  	const unsigned long be_to_le = BITS_PER_LONG - 1;
6592d0d634dacf Matthew Wilcox (Oracle  2021-04-09  76) 	bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  77  }
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  78  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31332 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep
  2021-04-09  2:51 ` [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep Matthew Wilcox (Oracle)
@ 2021-04-09  6:37   ` kernel test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-04-09  6:37 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle),
	neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy
  Cc: kbuild-all, Matthew Wilcox (Oracle), linux-kernel

[-- Attachment #1: Type: text/plain, Size: 10494 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210408]
[cannot apply to s390/features tip/x86/core dm/for-next gfs2/for-next block/for-next linus/master hnaz-linux-mm/master v5.12-rc6 v5.12-rc5 v5.12-rc4 v5.12-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Matthew-Wilcox-Oracle/Provide-lockdep-tracking-for-bit-spin-locks/20210409-110522
base:    6145d80cfc62e3ed8f16ff584d6287e6d88b82b9
config: s390-randconfig-r012-20210409 (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/49054b182384e0b597db2cf81e733efbf67c89b7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/Provide-lockdep-tracking-for-bit-spin-locks/20210409-110522
        git checkout 49054b182384e0b597db2cf81e733efbf67c89b7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h:73:20: error: 'airq_iv_lock' redeclared as different kind of symbol
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h: In function 'airq_iv_lock':
>> arch/s390/include/asm/airq.h:76:45: error: passing argument 3 of 'bit_spin_lock' from incompatible pointer type [-Werror=incompatible-pointer-types]
      76 |  bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
         |                                             ^~~~~~~~~~~~~
         |                                             |
         |                                             void (*)(struct airq_iv *, long unsigned int)
   In file included from include/linux/mm.h:22,
                    from include/linux/scatterlist.h:8,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1464,
                    from arch/s390/include/asm/hw_irq.h:6,
                    from include/linux/irq.h:589,
                    from drivers/s390/cio/airq.c:13:
   include/linux/bit_spinlock.h:42:22: note: expected 'struct split_lock *' but argument is of type 'void (*)(struct airq_iv *, long unsigned int)'
      42 |   struct split_lock *lock)
         |   ~~~~~~~~~~~~~~~~~~~^~~~
   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h: In function 'airq_iv_unlock':
>> arch/s390/include/asm/airq.h:82:47: error: passing argument 3 of 'bit_spin_unlock' from incompatible pointer type [-Werror=incompatible-pointer-types]
      82 |  bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
         |                                               ^~~~~~~~~~~~~
         |                                               |
         |                                               void (*)(struct airq_iv *, long unsigned int)
   In file included from include/linux/mm.h:22,
                    from include/linux/scatterlist.h:8,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1464,
                    from arch/s390/include/asm/hw_irq.h:6,
                    from include/linux/irq.h:589,
                    from drivers/s390/cio/airq.c:13:
   include/linux/bit_spinlock.h:69:22: note: expected 'struct split_lock *' but argument is of type 'void (*)(struct airq_iv *, long unsigned int)'
      69 |   struct split_lock *lock)
         |   ~~~~~~~~~~~~~~~~~~~^~~~
   In file included from include/linux/bit_spinlock.h:9,
                    from include/linux/mm.h:22,
                    from include/linux/scatterlist.h:8,
                    from include/linux/dmapool.h:14,
                    from include/linux/pci.h:1464,
                    from arch/s390/include/asm/hw_irq.h:6,
                    from include/linux/irq.h:589,
                    from drivers/s390/cio/airq.c:13:
   drivers/s390/cio/airq.c: At top level:
   drivers/s390/cio/airq.c:34:19: error: 'airq_iv_lock' redeclared as different kind of symbol
      34 | DEFINE_SPLIT_LOCK(airq_iv_lock);
         |                   ^~~~~~~~~~~~
   include/linux/split_lock.h:23:19: note: in definition of macro 'DEFINE_SPLIT_LOCK'
      23 | struct split_lock name = {      \
         |                   ^~~~
   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h:73:20: note: previous definition of 'airq_iv_lock' was here
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   In file included from include/linux/linkage.h:7,
                    from include/linux/preempt.h:10,
                    from include/linux/spinlock.h:51,
                    from include/linux/irq.h:14,
                    from drivers/s390/cio/airq.c:13:
   drivers/s390/cio/airq.c:35:15: error: conflicting types for 'airq_iv_lock'
      35 | EXPORT_SYMBOL(airq_iv_lock);
         |               ^~~~~~~~~~~~
   include/linux/export.h:98:21: note: in definition of macro '___EXPORT_SYMBOL'
      98 |  extern typeof(sym) sym;       \
         |                     ^~~
   include/linux/export.h:155:34: note: in expansion of macro '__EXPORT_SYMBOL'
     155 | #define _EXPORT_SYMBOL(sym, sec) __EXPORT_SYMBOL(sym, sec, "")
         |                                  ^~~~~~~~~~~~~~~
   include/linux/export.h:158:29: note: in expansion of macro '_EXPORT_SYMBOL'
     158 | #define EXPORT_SYMBOL(sym)  _EXPORT_SYMBOL(sym, "")
         |                             ^~~~~~~~~~~~~~
   drivers/s390/cio/airq.c:35:1: note: in expansion of macro 'EXPORT_SYMBOL'
      35 | EXPORT_SYMBOL(airq_iv_lock);
         | ^~~~~~~~~~~~~
   In file included from drivers/s390/cio/airq.c:21:
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from drivers/s390/cio/cio.c:30:
   arch/s390/include/asm/airq.h:73:20: error: 'airq_iv_lock' redeclared as different kind of symbol
      73 | static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
         |                    ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h:29:23: note: previous declaration of 'airq_iv_lock' was here
      29 | extern struct iv_lock airq_iv_lock;
         |                       ^~~~~~~~~~~~
   arch/s390/include/asm/airq.h: In function 'airq_iv_lock':
>> arch/s390/include/asm/airq.h:76:45: error: passing argument 3 of 'bit_spin_lock' from incompatible pointer type [-Werror=incompatible-pointer-types]
      76 |  bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
         |                                             ^~~~~~~~~~~~~
         |                                             |
         |                                             void (*)(struct airq_iv *, long unsigned int)
   In file included from include/linux/mm.h:22,
                    from include/linux/kallsyms.h:12,
                    from include/linux/ftrace.h:12,
                    from drivers/s390/cio/cio.c:15:
   include/linux/bit_spinlock.h:42:22: note: expected 'struct split_lock *' but argument is of type 'void (*)(struct airq_iv *, long unsigned int)'
      42 |   struct split_lock *lock)
         |   ~~~~~~~~~~~~~~~~~~~^~~~
   In file included from drivers/s390/cio/cio.c:30:
   arch/s390/include/asm/airq.h: In function 'airq_iv_unlock':
>> arch/s390/include/asm/airq.h:82:47: error: passing argument 3 of 'bit_spin_unlock' from incompatible pointer type [-Werror=incompatible-pointer-types]
      82 |  bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
         |                                               ^~~~~~~~~~~~~
         |                                               |
         |                                               void (*)(struct airq_iv *, long unsigned int)
   In file included from include/linux/mm.h:22,
                    from include/linux/kallsyms.h:12,
                    from include/linux/ftrace.h:12,
                    from drivers/s390/cio/cio.c:15:
   include/linux/bit_spinlock.h:69:22: note: expected 'struct split_lock *' but argument is of type 'void (*)(struct airq_iv *, long unsigned int)'
      69 |   struct split_lock *lock)
         |   ~~~~~~~~~~~~~~~~~~~^~~~
   cc1: some warnings being treated as errors


vim +/bit_spin_lock +76 arch/s390/include/asm/airq.h

a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  72  
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  73  static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  74  {
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  75  	const unsigned long be_to_le = BITS_PER_LONG - 1;
6592d0d634dacf Matthew Wilcox (Oracle  2021-04-09 @76) 	bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  77  }
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  78  
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  79  static inline void airq_iv_unlock(struct airq_iv *iv, unsigned long bit)
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  80  {
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  81  	const unsigned long be_to_le = BITS_PER_LONG - 1;
6592d0d634dacf Matthew Wilcox (Oracle  2021-04-09 @82) 	bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  83  }
a9a6f0341df9a6 Martin Schwidefsky      2013-06-25  84  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31332 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 11/17] s390: Add airq_iv_lock
  2021-04-09  6:18   ` kernel test robot
@ 2021-04-09 13:20     ` Matthew Wilcox
  0 siblings, 0 replies; 28+ messages in thread
From: Matthew Wilcox @ 2021-04-09 13:20 UTC (permalink / raw)
  To: kernel test robot
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy,
	kbuild-all, linux-kernel

On Fri, Apr 09, 2021 at 02:18:16PM +0800, kernel test robot wrote:
> Hi "Matthew,
> 
> Thank you for the patch! Yet something to improve:

Thanks.  I'll fold in this patch to fix it.


diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index d281340de14a..26030b7c1b88 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -26,7 +26,7 @@ struct airq_struct {
 
 int register_adapter_interrupt(struct airq_struct *airq);
 void unregister_adapter_interrupt(struct airq_struct *airq);
-extern struct iv_lock airq_iv_lock;
+extern struct split_lock airq_split_lock;
 
 /* Adapter interrupt bit vector */
 struct airq_iv {
@@ -73,13 +73,13 @@ static inline unsigned long airq_iv_end(struct airq_iv *iv)
 static inline void airq_iv_lock(struct airq_iv *iv, unsigned long bit)
 {
 	const unsigned long be_to_le = BITS_PER_LONG - 1;
-	bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
+	bit_spin_lock(bit ^ be_to_le, iv->bitlock, &airq_split_lock);
 }
 
 static inline void airq_iv_unlock(struct airq_iv *iv, unsigned long bit)
 {
 	const unsigned long be_to_le = BITS_PER_LONG - 1;
-	bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_iv_lock);
+	bit_spin_unlock(bit ^ be_to_le, iv->bitlock, &airq_split_lock);
 }
 
 static inline void airq_iv_set_data(struct airq_iv *iv, unsigned long bit,
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index 6e850661957c..08bb47200f12 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -31,8 +31,8 @@ static struct hlist_head airq_lists[MAX_ISC+1];
 
 static struct dma_pool *airq_iv_cache;
 
-DEFINE_SPLIT_LOCK(airq_iv_lock);
-EXPORT_SYMBOL(airq_iv_lock);
+DEFINE_SPLIT_LOCK(airq_split_lock);
+EXPORT_SYMBOL(airq_split_lock);
 
 /**
  * register_adapter_interrupt() - register adapter interrupt handler



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 03/17] bit_spinlock: Prepare for split_locks
  2021-04-09  2:51 ` [PATCH 03/17] bit_spinlock: Prepare for split_locks Matthew Wilcox (Oracle)
@ 2021-04-09 14:32   ` Theodore Ts'o
  2021-04-09 14:35     ` Matthew Wilcox
  0 siblings, 1 reply; 28+ messages in thread
From: Theodore Ts'o @ 2021-04-09 14:32 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy,
	linux-kernel

On Fri, Apr 09, 2021 at 03:51:17AM +0100, Matthew Wilcox (Oracle) wrote:
> Make bit_spin_lock() and variants variadic to help with the transition.
> The split_lock parameter will become mandatory at the end of the series.
> Also add bit_spin_lock_nested() and bit_spin_unlock_assign() which will
> both be used by the rhashtable code later.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

This changes the function signature for bit_spin_lock(), if I'm
reading this correctly.  Hence, this is going to break git
bisectability; was this patch series separated out for easy of review,
and you were planning on collapsing things into a single patch to
preserve bisectability?

					- Ted

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 03/17] bit_spinlock: Prepare for split_locks
  2021-04-09 14:32   ` Theodore Ts'o
@ 2021-04-09 14:35     ` Matthew Wilcox
  2021-04-09 14:55       ` Theodore Ts'o
  0 siblings, 1 reply; 28+ messages in thread
From: Matthew Wilcox @ 2021-04-09 14:35 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy,
	linux-kernel

On Fri, Apr 09, 2021 at 10:32:47AM -0400, Theodore Ts'o wrote:
> On Fri, Apr 09, 2021 at 03:51:17AM +0100, Matthew Wilcox (Oracle) wrote:
> > Make bit_spin_lock() and variants variadic to help with the transition.
> > The split_lock parameter will become mandatory at the end of the series.
> > Also add bit_spin_lock_nested() and bit_spin_unlock_assign() which will
> > both be used by the rhashtable code later.
> > 
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> This changes the function signature for bit_spin_lock(), if I'm
> reading this correctly.  Hence, this is going to break git
> bisectability; was this patch series separated out for easy of review,
> and you were planning on collapsing things into a single patch to
> preserve bisectability?

It's perfectly bisectable.

Before: bit_spin_lock takes two arguments
During: bit_spin_lock takes at least two arguments, ignores all but the first two
After: bit_spin_lock takes three arguments

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 03/17] bit_spinlock: Prepare for split_locks
  2021-04-09 14:35     ` Matthew Wilcox
@ 2021-04-09 14:55       ` Theodore Ts'o
  0 siblings, 0 replies; 28+ messages in thread
From: Theodore Ts'o @ 2021-04-09 14:55 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, tglx, bigeasy,
	linux-kernel

On Fri, Apr 09, 2021 at 03:35:55PM +0100, Matthew Wilcox wrote:
> > This changes the function signature for bit_spin_lock(), if I'm
> > reading this correctly.  Hence, this is going to break git
> > bisectability; was this patch series separated out for easy of review,
> > and you were planning on collapsing things into a single patch to
> > preserve bisectability?
> 
> It's perfectly bisectable.
> 
> Before: bit_spin_lock takes two arguments
> During: bit_spin_lock takes at least two arguments, ignores all but the first two
> After: bit_spin_lock takes three arguments

Ah, got it, thanks for the clarification!

					- Ted

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/17] locking: Add split_lock
  2021-04-09  2:51 ` [PATCH 02/17] locking: Add split_lock Matthew Wilcox (Oracle)
@ 2021-04-12 14:29   ` Thomas Gleixner
  2021-04-12 14:45     ` Matthew Wilcox
  0 siblings, 1 reply; 28+ messages in thread
From: Thomas Gleixner @ 2021-04-12 14:29 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, bigeasy, linux-kernel

On Fri, Apr 09 2021 at 03:51, Matthew Wilcox wrote:
> Bitlocks do not currently participate in lockdep.  Conceptually, a
> bit_spinlock is a split lock, eg across each bucket in a hash table.
> The struct split_lock gives us somewhere to record the lockdep_map.

I like the concept, but the name is strange. The only purpose of 

> +struct split_lock {
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +	struct lockdep_map dep_map;
> +#endif
> +};

is to have a place to stick the lockdep map into. So it's not a lock
construct as the name suggests, it's just auxiliary data when lockdep is
enabled.

I know you hinted that RT could make use of that data structure and the
fact that it's mandatory for the various lock functions, but that's not
really feasible if this is related to a hash with a bit spinlock per
bucket as the data structure is hash global.

Sure, we can do pointer math to find out the bucket index and do
something from there, but I'm not sure whether that really helps. Need
to stare at the remaining few places where bit spinlocks are an issue on
RT.

Thanks,

        tglx



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/17] locking: Add split_lock
  2021-04-12 14:29   ` Thomas Gleixner
@ 2021-04-12 14:45     ` Matthew Wilcox
  2021-04-12 15:01       ` Thomas Gleixner
  2021-05-11  7:46       ` Peter Zijlstra
  0 siblings, 2 replies; 28+ messages in thread
From: Matthew Wilcox @ 2021-04-12 14:45 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, bigeasy, linux-kernel

On Mon, Apr 12, 2021 at 04:29:28PM +0200, Thomas Gleixner wrote:
> On Fri, Apr 09 2021 at 03:51, Matthew Wilcox wrote:
> > Bitlocks do not currently participate in lockdep.  Conceptually, a
> > bit_spinlock is a split lock, eg across each bucket in a hash table.
> > The struct split_lock gives us somewhere to record the lockdep_map.
> 
> I like the concept, but the name is strange. The only purpose of 
> 
> > +struct split_lock {
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > +	struct lockdep_map dep_map;
> > +#endif
> > +};
> 
> is to have a place to stick the lockdep map into. So it's not a lock
> construct as the name suggests, it's just auxiliary data when lockdep is
> enabled.

That's the implementation _today_, but conceptually, it's a single lock.
I was thinking that for non-RT, we could put a qspinlock in there for a
thread to spin on if the bit is contended.  It'd need a bit of ingenuity
to make sure that a thread unlocking a bitlock made sure that a thread
spinning on the qspinlock saw the wakeup, but it should be doable.

Anyway, from the point of view of the user, they should be declaring
"this is the lock", not "this is the lock tracking structure", right?

> I know you hinted that RT could make use of that data structure and the
> fact that it's mandatory for the various lock functions, but that's not
> really feasible if this is related to a hash with a bit spinlock per
> bucket as the data structure is hash global.
> 
> Sure, we can do pointer math to find out the bucket index and do
> something from there, but I'm not sure whether that really helps. Need
> to stare at the remaining few places where bit spinlocks are an issue on
> RT.

I obviously don't understand exactly what the RT patchset does.  My
thinking was that you could handle the bit locks like rw sems, and
sacrifice the scalability of per-bucket-lock for the determinism of
a single PI lock.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/17] locking: Add split_lock
  2021-04-12 14:45     ` Matthew Wilcox
@ 2021-04-12 15:01       ` Thomas Gleixner
  2021-05-11  7:46       ` Peter Zijlstra
  1 sibling, 0 replies; 28+ messages in thread
From: Thomas Gleixner @ 2021-04-12 15:01 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: neilb, peterz, mingo, will, longman, boqun.feng, bigeasy, linux-kernel

On Mon, Apr 12 2021 at 15:45, Matthew Wilcox wrote:
> On Mon, Apr 12, 2021 at 04:29:28PM +0200, Thomas Gleixner wrote:
>> On Fri, Apr 09 2021 at 03:51, Matthew Wilcox wrote:
>> > Bitlocks do not currently participate in lockdep.  Conceptually, a
>> > bit_spinlock is a split lock, eg across each bucket in a hash table.
>> > The struct split_lock gives us somewhere to record the lockdep_map.
>> 
>> I like the concept, but the name is strange. The only purpose of 
>> 
>> > +struct split_lock {
>> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
>> > +	struct lockdep_map dep_map;
>> > +#endif
>> > +};
>> 
>> is to have a place to stick the lockdep map into. So it's not a lock
>> construct as the name suggests, it's just auxiliary data when lockdep is
>> enabled.
>
> That's the implementation _today_, but conceptually, it's a single lock.
> I was thinking that for non-RT, we could put a qspinlock in there for a
> thread to spin on if the bit is contended.  It'd need a bit of ingenuity
> to make sure that a thread unlocking a bitlock made sure that a thread
> spinning on the qspinlock saw the wakeup, but it should be doable.

Ah, that's what you have in mind.

> Anyway, from the point of view of the user, they should be declaring
> "this is the lock", not "this is the lock tracking structure", right?
>
>> I know you hinted that RT could make use of that data structure and the
>> fact that it's mandatory for the various lock functions, but that's not
>> really feasible if this is related to a hash with a bit spinlock per
>> bucket as the data structure is hash global.
>> 
>> Sure, we can do pointer math to find out the bucket index and do
>> something from there, but I'm not sure whether that really helps. Need
>> to stare at the remaining few places where bit spinlocks are an issue on
>> RT.
>
> I obviously don't understand exactly what the RT patchset does.  My
> thinking was that you could handle the bit locks like rw sems, and
> sacrifice the scalability of per-bucket-lock for the determinism of
> a single PI lock.

That'd suck for most bit spinlocks where the lock is just protecting
minimal hashlist operations and these preeempt disabled protections are
actually shorter than the overhead of a heavier lock.

For situations where the bit spinlock was actually an issue (long
traversals or such) in older kernel versions we just bit the bullet and
bloated the hash data structure with an actual spinlock and had some
wrappers to hide the mess from the actual code. That still preserved the
scalability while making the lock held section preemptible which we
obviously cannot have with real bit spinlocks even on RT.

But your idea of having a qspinlock for the contended case might
actually be something which might be worth to exploit RT wise -
obviously not with a qspinlock :) - but conceptually.

Need to think more about it.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 02/17] locking: Add split_lock
  2021-04-12 14:45     ` Matthew Wilcox
  2021-04-12 15:01       ` Thomas Gleixner
@ 2021-05-11  7:46       ` Peter Zijlstra
  1 sibling, 0 replies; 28+ messages in thread
From: Peter Zijlstra @ 2021-05-11  7:46 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Thomas Gleixner, neilb, mingo, will, longman, boqun.feng,
	bigeasy, linux-kernel

On Mon, Apr 12, 2021 at 03:45:25PM +0100, Matthew Wilcox wrote:
> On Mon, Apr 12, 2021 at 04:29:28PM +0200, Thomas Gleixner wrote:
> > is to have a place to stick the lockdep map into. So it's not a lock
> > construct as the name suggests, it's just auxiliary data when lockdep is
> > enabled.
> 
> That's the implementation _today_, but conceptually, it's a single lock.
> I was thinking that for non-RT, we could put a qspinlock in there for a
> thread to spin on if the bit is contended.  It'd need a bit of ingenuity
> to make sure that a thread unlocking a bitlock made sure that a thread
> spinning on the qspinlock saw the wakeup, but it should be doable.

queued_write_lock_slowpath() does more or less exactly what you
describe.

I just worry about loss of concurrency if we were to do that. Where
currently we could be spinning on 5 different hash buckets and make
individual progress, doing what you propose would limit that.

Imagine having one bit-spinlock taken and another cpu contending, it
would go into the queue. Then do the same with another bit-spinlock,
with another two CPUs, the second again goes into that same queue.

So now we have 2 CPUs owning a bit-spinlock, and 2 CPUs stuck in the
queue. Suppose the second bit-spinlock is released, this would make the
queue-tail elegible to aquire, but it's stuck behind the queue-head
which is still waiting for its bit-spinlock. So it'll stay queued and we
loose concurrency.

Anyway, I think all this is worthwhile just to get bit-spinlock lockdep
coverage. And it's not like we can't change any of this when/if we get a
better idea or something.


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-05-11  7:46 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09  2:51 [PATCH 00/17] Provide lockdep tracking for bit spin locks Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 01/17] x86: Rename split_lock_init to sld_init Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 02/17] locking: Add split_lock Matthew Wilcox (Oracle)
2021-04-12 14:29   ` Thomas Gleixner
2021-04-12 14:45     ` Matthew Wilcox
2021-04-12 15:01       ` Thomas Gleixner
2021-05-11  7:46       ` Peter Zijlstra
2021-04-09  2:51 ` [PATCH 03/17] bit_spinlock: Prepare for split_locks Matthew Wilcox (Oracle)
2021-04-09 14:32   ` Theodore Ts'o
2021-04-09 14:35     ` Matthew Wilcox
2021-04-09 14:55       ` Theodore Ts'o
2021-04-09  2:51 ` [PATCH 04/17] hlist_bl: " Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 05/17] dm-snap: Add dm_exceptional_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 06/17] dcache: Add d_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 07/17] fscache: Add cookie_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 08/17] gfs2: Add qd_hash_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 09/17] mbcache: Add mb_cache_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 10/17] hlist_bl: Make the split_lock parameter mandatory Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 11/17] s390: Add airq_iv_lock Matthew Wilcox (Oracle)
2021-04-09  6:18   ` kernel test robot
2021-04-09 13:20     ` Matthew Wilcox
2021-04-09  2:51 ` [PATCH 12/17] zram: Add zram_table_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 13/17] jbd2: Add jbd2_jh_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 14/17] slub: Add slab_page_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 15/17] zsmalloc: Add zs_pin_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 16/17] rhashtable: Convert to split_lock Matthew Wilcox (Oracle)
2021-04-09  2:51 ` [PATCH 17/17] bit_spinlock: Track bit spin locks with lockdep Matthew Wilcox (Oracle)
2021-04-09  6:37   ` kernel test robot

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).