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
I want to use split_lock_init() for a global symbol, so rename this
local one.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
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) <[email protected]>
---
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
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) <[email protected]>
---
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
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) <[email protected]>
---
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
Allow lockdep to track the dm-snap bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track the d_hash bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track the fscache cookie hash bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track the hash bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track the mbcache hash bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
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) <[email protected]>
---
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
Allow lockdep to track the airq bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track the journal bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track slub's page bit spin lock.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
Allow lockdep to track zsmalloc's pin bit spin lock.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
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) <[email protected]>
Cc: NeilBrown <[email protected]>
---
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
Allow lockdep to track the zram bit spin locks.
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
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
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) <[email protected]>
---
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
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 <[email protected]>
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/[email protected]
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 <[email protected]>
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/[email protected]
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
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) <[email protected]>
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
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) <[email protected]>
>
> 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
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
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.
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
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
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.