Re: [PATCHv4 09/12] x86/mm: Provide pmdp_establish() helper

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Wed, Dec 13, 2017 at 04:09:51PM -0800, Andrew Morton wrote:
> > @@ -181,6 +182,40 @@ static inline pmd_t native_pmdp_get_and_clear(pmd_t *pmdp)
> >  #define native_pmdp_get_and_clear(xp) native_local_pmdp_get_and_clear(xp)
> >  #endif
> >  
> > +#ifndef pmdp_establish
> > +#define pmdp_establish pmdp_establish
> > +static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
> > +		unsigned long address, pmd_t *pmdp, pmd_t pmd)
> > +{
> > +	pmd_t old;
> > +
> > +	/*
> > +	 * If pmd has present bit cleared we can get away without expensive
> > +	 * cmpxchg64: we can update pmdp half-by-half without racing with
> > +	 * anybody.
> > +	 */
> > +	if (!(pmd_val(pmd) & _PAGE_PRESENT)) {
> > +		union split_pmd old, new, *ptr;
> > +
> > +		ptr = (union split_pmd *)pmdp;
> > +
> > +		new.pmd = pmd;
> > +
> > +		/* xchg acts as a barrier before setting of the high bits */
> > +		old.pmd_low = xchg(&ptr->pmd_low, new.pmd_low);
> > +		old.pmd_high = ptr->pmd_high;
> > +		ptr->pmd_high = new.pmd_high;
> > +		return old.pmd;
> > +	}
> > +
> > +	{
> > +		old = *pmdp;
> > +	} while (cmpxchg64(&pmdp->pmd, old.pmd, pmd.pmd) != old.pmd);
> 
> um, what happened here?

Ouch.. Yeah, we need 'do' here. :-/

Apparently, it's a valid C code that would run the body once and it worked for
me because I didn't hit the race condition.

-- 
 Kirill A. Shutemov