linux-kernel - Re: [PATCH 05/10] staging: erofs: add a full barrier in erofs_workgroup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181122185058.GA3466@andrea>
Date:   Thu, 22 Nov 2018 19:50:58 +0100
From:   Andrea Parri <andrea.parri@...rulasolutions.com>
To:     Gao Xiang <gaoxiang25@...wei.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, linux-erofs@...ts.ozlabs.org,
        Chao Yu <yuchao0@...wei.com>,
        LKML <linux-kernel@...r.kernel.org>, weidu.du@...wei.com,
        Miao Xie <miaoxie@...wei.com>
Subject: Re: [PATCH 05/10] staging: erofs: add a full barrier in
 erofs_workgroup_unfreeze

On Thu, Nov 22, 2018 at 06:56:32PM +0800, Gao Xiang wrote:
> Hi Greg,
> 
> On 2018/11/22 18:22, Greg Kroah-Hartman wrote:
> > Please document this memory barrier.  It does not make much sense to
> > me...
> 
> Because we need to make the other observers noticing the latest values modified
> in this locking period before unfreezing the whole workgroup, one way is to use
> a memory barrier and the other way is to use ACQUIRE and RELEASE. we selected
> the first one.
> 
> Hmmm...ok, I will add a simple message to explain this, but I think that is
> plain enough for a lock...

Sympathizing with Greg's request, let me add some specific suggestions:

  1. It wouldn't hurt to indicate a pair of memory accesses which are
     intended to be "ordered" by the memory barrier in question (yes,
     this pair might not be unique, but you should be able to provide
     an example).

  2. Memory barriers always come matched by other memory barriers, or
     dependencies (it really does not make sense to talk about a full
     barrier "in isolation"): please also indicate (an instance of) a
     matching barrier or the matching barriers.

  3. How do the hardware threads communicate?  In the acquire/release
     pattern you mentioned above, the load-acquire *reads from* a/the
     previous store-release, a memory access that follows the acquire
     somehow communicate with a memory access preceding the release...

  4. It is a good practice to include the above information within an
     (inline) comment accompanying the added memory barrier (in fact,
     IIRC, checkpatch.pl gives you a "memory barrier without comment"
     warning when you omit to do so); not just in the commit message.

Hope this helps.  Please let me know if something I wrote is unclear,

  Andrea


> 
> Thanks,
> Gao Xiang
> 
> > 
> > thanks,
> > 
> > greg k-h