RE: [PATCH] netfilter: nf_conntrack: use safer way to lock all buckets

[David Laight]

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);
> +	}
> +}
...
> @@ -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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/