From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1756968AbcH2Kvc (ORCPT <rfc822;w@1wt.eu>);
        Mon, 29 Aug 2016 06:51:32 -0400
Received: from merlin.infradead.org ([205.233.59.134]:39616 "EHLO
        merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750909AbcH2Kvb (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 29 Aug 2016 06:51:31 -0400
Date: Mon, 29 Aug 2016 12:51:22 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: benh@kernel.crashing.org, paulmck@linux.vnet.ibm.com,
        Ingo Molnar <mingo@elte.hu>, Boqun Feng <boqun.feng@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, 1vier1@web.de,
        Davidlohr Bueso <dave@stgolabs.net>
Subject: Re: [PATCH 3/4] net/netfilter/nf_conntrack_core: update memory
 barriers.
Message-ID: <20160829105122.GJ10153@twins.programming.kicks-ass.net>
References: <1472385376-8801-1-git-send-email-manfred@colorfullife.com>
 <1472385376-8801-2-git-send-email-manfred@colorfullife.com>
 <1472385376-8801-3-git-send-email-manfred@colorfullife.com>
 <1472385376-8801-4-git-send-email-manfred@colorfullife.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1472385376-8801-4-git-send-email-manfred@colorfullife.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sun, Aug 28, 2016 at 01:56:15PM +0200, Manfred Spraul wrote:
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -80,20 +80,29 @@ static __read_mostly bool nf_conntrack_locks_all;
>  
>  void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
>  {
> +	/* Step 1: Acquire the lock */
>  	spin_lock(lock);
> -	while (unlikely(nf_conntrack_locks_all)) {
> -		spin_unlock(lock);
>  
> -		/*
> -		 * Order the 'nf_conntrack_locks_all' load vs. the
> -		 * spin_unlock_wait() loads below, to ensure
> -		 * that 'nf_conntrack_locks_all_lock' is indeed held:
> -		 */
> -		smp_rmb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
> -		spin_unlock_wait(&nf_conntrack_locks_all_lock);
> -		spin_lock(lock);
> -	}
> +	/* Step 2: make it visible to all CPUs that we hold the lock */
> +	smp_mb__after_spin_lock();

I hate this comment. A barrier does _not_ make visible anything.

A barrier _orders_ things.