linux-kernel - Re: [PATCH 2/3] mm/page_ref: Ensure page_ref_unfreeze is ordered against prior accesses

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] [day] [month] [year] [list]

Message-ID: <20170609100529.GD13955@arm.com>
Date:   Fri, 9 Jun 2017 11:05:29 +0100
From:   Will Deacon <will.deacon@....com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Vlastimil Babka <vbabka@...e.cz>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        mark.rutland@....com, akpm@...ux-foundation.org,
        Punit.Agrawal@....com, mgorman@...e.de, steve.capper@....com
Subject: Re: [PATCH 2/3] mm/page_ref: Ensure page_ref_unfreeze is ordered
 against prior accesses

On Thu, Jun 08, 2017 at 02:50:59PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 08, 2017 at 12:24:33PM +0100, Will Deacon wrote:
> > [+ PeterZ]
> > 
> > On Thu, Jun 08, 2017 at 01:07:02PM +0200, Vlastimil Babka wrote:
> > > On 06/08/2017 12:40 PM, Kirill A. Shutemov wrote:
> > > > On Thu, Jun 08, 2017 at 11:38:21AM +0200, Vlastimil Babka wrote:
> > > >> On 06/06/2017 07:58 PM, Will Deacon wrote:
> > > >>>  include/linux/page_ref.h | 1 +
> > > >>>  1 file changed, 1 insertion(+)
> > > >>>
> > > >>> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> > > >>> index 610e13271918..74d32d7905cb 100644
> > > >>> --- a/include/linux/page_ref.h
> > > >>> +++ b/include/linux/page_ref.h
> > > >>> @@ -174,6 +174,7 @@ static inline void page_ref_unfreeze(struct page *page, int count)
> > > >>>  	VM_BUG_ON_PAGE(page_count(page) != 0, page);
> > > >>>  	VM_BUG_ON(count == 0);
> > > >>>  
> > > >>> +	smp_mb__before_atomic();
> > > >>>  	atomic_set(&page->_refcount, count);
> 
> So depending on what it actually required, we do have
> atomic_set_release() (atomic_t equivalent to smp_store_release()).

Yeah, I was wondering about that this morning. I think it should do the
trick here, but smp_mb() would be a better fit for the other parts of this
API (page_ref_freeze uses atomic_cmpxchg and page_cache_get_speculative
uses atomic_add_unless).

I'll send a v2 with the full barrier.

Will