RE: [PATCH] netfilter: nf_conntrack: use safer way to lock all buckets
From: David Laight <hidden>
Date: 2016-01-05 11:16:34
Also in:
lkml, netfilter-devel
From: Sasha Levin
Sent: 05 January 2016 02:26 When we need to lock all buckets in the connection hashtable we'd attempt to lock 1024 spinlocks, which is way more preemption levels than supported by the kernel. Furthermore, this behavior was hidden by checking if lockdep is enabled, and if it was - use only 8 buckets(!). Fix this by using a global lock and synchronize all buckets on it when we need to lock them all. This is pretty heavyweight, but is only done when we need to resize the hashtable, and that doesn't happen often enough (or at all).
...
+static void nf_conntrack_lock_nested(spinlock_t *lock)
+{
+ spin_lock_nested(lock, SINGLE_DEPTH_NESTING);
+ while (unlikely(nf_conntrack_locks_all)) {
+ spin_unlock(lock);
+ spin_lock(&nf_conntrack_locks_all_lock);
+ spin_unlock(&nf_conntrack_locks_all_lock);
+ spin_lock_nested(lock, SINGLE_DEPTH_NESTING);
+ }
+}...
quoted hunk ↗ jump to hunk
@@ -102,16 +126,19 @@ static void nf_conntrack_all_lock(void) { int i; - for (i = 0; i < CONNTRACK_LOCKS; i++) - spin_lock_nested(&nf_conntrack_locks[i], i); + spin_lock(&nf_conntrack_locks_all_lock); + nf_conntrack_locks_all = true; + + for (i = 0; i < CONNTRACK_LOCKS; i++) { + spin_lock(&nf_conntrack_locks[i]); + spin_unlock(&nf_conntrack_locks[i]); + } }
If spin_lock_nested() does anything like what I think its name suggests then I suspect that deadlocks. David