Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] btrfs: optimize barrier usage for Rmw atomics
To:     Davidlohr Bueso <dave@stgolabs.net>, dsterba@suse.cz,
        dsterba@suse.com, nborisov@suse.com, linux-btrfs@vger.kernel.org,
        linux-kernel@vger.kernel.org, Davidlohr Bueso <dbueso@suse.de>
References: <20200129180324.24099-1-dave@stgolabs.net>
 <20200129191439.GN3929@twin.jikos.cz>
 <20200129192550.nnfkkgde445nrbko@linux-p48b>
From:   Qu Wenruo <quwenruo.btrfs@gmx.com>
Autocrypt: addr=quwenruo.btrfs@gmx.com; prefer-encrypt=mutual; keydata=
 mQENBFnVga8BCACyhFP3ExcTIuB73jDIBA/vSoYcTyysFQzPvez64TUSCv1SgXEByR7fju3o
 8RfaWuHCnkkea5luuTZMqfgTXrun2dqNVYDNOV6RIVrc4YuG20yhC1epnV55fJCThqij0MRL
 1NxPKXIlEdHvN0Kov3CtWA+R1iNN0RCeVun7rmOrrjBK573aWC5sgP7YsBOLK79H3tmUtz6b
 9Imuj0ZyEsa76Xg9PX9Hn2myKj1hfWGS+5og9Va4hrwQC8ipjXik6NKR5GDV+hOZkktU81G5
 gkQtGB9jOAYRs86QG/b7PtIlbd3+pppT0gaS+wvwMs8cuNG+Pu6KO1oC4jgdseFLu7NpABEB
 AAG0IlF1IFdlbnJ1byA8cXV3ZW5ydW8uYnRyZnNAZ214LmNvbT6JAU4EEwEIADgCGwMFCwkI
 BwIGFQgJCgsCBBYCAwECHgECF4AWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCXZw1oQAKCRDC
 PZHzoSX+qCY6CACd+mWu3okGwRKXju6bou+7VkqCaHTdyXwWFTsr+/0ly5nUdDtT3yEVggPJ
 3VP70wjlrxUjNjFb6iIvGYxiPOrop1NGwGYvQktgRhaIhALG6rPoSSAhGNjwGVRw0km0PlIN
 D29BTj/lYEk+jVM1YL0QLgAE1AI3krihg/lp/fQT53wLhR8YZIF8ETXbClQG1vJ0cllPuEEv
 efKxRyiTSjB+PsozSvYWhXsPeJ+KKjFen7ebE5reQTPFzSHctCdPnoR/4jSPlnTlnEvLeqcD
 ZTuKfQe1gWrPeevQzgCtgBF/WjIOeJs41klnYzC3DymuQlmFubss0jShLOW8eSOOWhLRuQEN
 BFnVga8BCACqU+th4Esy/c8BnvliFAjAfpzhI1wH76FD1MJPmAhA3DnX5JDORcgaCbPEwhLj
 1xlwTgpeT+QfDmGJ5B5BlrrQFZVE1fChEjiJvyiSAO4yQPkrPVYTI7Xj34FnscPj/IrRUUka
 68MlHxPtFnAHr25VIuOS41lmYKYNwPNLRz9Ik6DmeTG3WJO2BQRNvXA0pXrJH1fNGSsRb+pK
 EKHKtL1803x71zQxCwLh+zLP1iXHVM5j8gX9zqupigQR/Cel2XPS44zWcDW8r7B0q1eW4Jrv
 0x19p4P923voqn+joIAostyNTUjCeSrUdKth9jcdlam9X2DziA/DHDFfS5eq4fEvABEBAAGJ
 ATwEGAEIACYCGwwWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCXZw1rgUJCWpOfwAKCRDCPZHz
 oSX+qFcEB/95cs8cM1OQdE/GgOfCGxwgckMeWyzOR7bkAWW0lDVp2hpgJuxBW/gyfmtBnUai
 fnggx3EE3ev8HTysZU9q0h+TJwwJKGv6sUc8qcTGFDtavnnl+r6xDUY7A6GvXEsSoCEEynby
 72byGeSovfq/4AWGNPBG1L61Exl+gbqfvbECP3ziXnob009+z9I4qXodHSYINfAkZkA523JG
 ap12LndJeLk3gfWNZfXEWyGnuciRGbqESkhIRav8ootsCIops/SqXm0/k+Kcl4gGUO/iD/T5
 oagaDh0QtOd8RWSMwLxwn8uIhpH84Q4X1LadJ5NCgGa6xPP5qqRuiC+9gZqbq4Nj
Message-ID: <f49aaafa-9144-5644-adae-d5bc13b6ca41@gmx.com>
Date:   Thu, 30 Jan 2020 07:55:35 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.2
MIME-Version: 1.0
In-Reply-To: <20200129192550.nnfkkgde445nrbko@linux-p48b>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2020/1/30 =E4=B8=8A=E5=8D=883:25, Davidlohr Bueso wrote:
> On Wed, 29 Jan 2020, David Sterba wrote:
>
>> On Wed, Jan 29, 2020 at 10:03:24AM -0800, Davidlohr Bueso wrote:
>>> Use smp_mb__after_atomic() instead of smp_mb() and avoid the
>>> unnecessary barrier for non LL/SC architectures, such as x86.
>>
>> So that's a conflicting advice from what we got when discussing wich
>> barriers to use in 6282675e6708ec78518cc0e9ad1f1f73d7c5c53d and the
>> memory is still fresh. My first idea was to take the
>> smp_mb__after_atomic and __before_atomic variants and after discussion
>> with various people the plain smp_wmb/smp_rmb were suggested and used i=
n
>> the end.
>
> So the patch you mention deals with test_bit(), which is out of the scop=
e
> of smp_mb__{before,after}_atomic() as it's not a RMW operation.
> atomic_inc()
> and set_bit() are, however, meant to use these barriers.

Exactly!
I'm still not convinced to use full barrier for test_bit() and I see no
reason to use any barrier for test_bit().
All mb should only be needed between two or more memory access, thus mb
should sit between set/clear_bit() and other operations, not around
test_bit().

>
>>
>> I can dig the email threads and excerpts from irc conversations, maybe
>> Nik has them at hand too. We do want to get rid of all unnecessary and
>> uncommented barriers in btrfs code, so I appreciate your patch.
>
> Yeah, I struggled with the amount of undocumented barriers, and decided
> not to go down that rabbit hole. This patch is only an equivalent of
> what is currently there. When possible, getting rid of barriers is of
> course better.

BTW, is there any convincing method to do proper mb examination?

I really found it hard to convince others or even myself when mb is
involved.

Thanks,
Qu

>
> Thanks,
> Davidlohr