Re: [PATCH] x86/mm: fix regression with huge pages on PAE

[Ingo Molnar]

* Kirill A. Shutemov <kirill@xxxxxxxxxxxxx> wrote:

> On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
> > 
> > * Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> > 
> > > diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
> > > index c5b7fb2774d0..cc071c6f7d4d 100644
> > > --- a/arch/x86/include/asm/page_types.h
> > > +++ b/arch/x86/include/asm/page_types.h
> > > @@ -9,19 +9,21 @@
> > >  #define PAGE_SIZE	(_AC(1,UL) << PAGE_SHIFT)
> > >  #define PAGE_MASK	(~(PAGE_SIZE-1))
> > >  
> > > +#define PMD_PAGE_SIZE		(_AC(1, UL) << PMD_SHIFT)
> > > +#define PMD_PAGE_MASK		(~(PMD_PAGE_SIZE-1))
> > > +
> > > +#define PUD_PAGE_SIZE		(_AC(1, UL) << PUD_SHIFT)
> > > +#define PUD_PAGE_MASK		(~(PUD_PAGE_SIZE-1))
> > > +
> > >  #define __PHYSICAL_MASK		((phys_addr_t)((1ULL << __PHYSICAL_MASK_SHIFT) - 1))
> > >  #define __VIRTUAL_MASK		((1UL << __VIRTUAL_MASK_SHIFT) - 1)
> > >  
> > > -/* Cast PAGE_MASK to a signed type so that it is sign-extended if
> > > +/* Cast *PAGE_MASK to a signed type so that it is sign-extended if
> > >     virtual addresses are 32-bits but physical addresses are larger
> > >     (ie, 32-bit PAE). */
> > >  #define PHYSICAL_PAGE_MASK	(((signed long)PAGE_MASK) & __PHYSICAL_MASK)
> > > -
> > > -#define PMD_PAGE_SIZE		(_AC(1, UL) << PMD_SHIFT)
> > > -#define PMD_PAGE_MASK		(~(PMD_PAGE_SIZE-1))
> > > -
> > > -#define PUD_PAGE_SIZE		(_AC(1, UL) << PUD_SHIFT)
> > > -#define PUD_PAGE_MASK		(~(PUD_PAGE_SIZE-1))
> > > +#define PHYSICAL_PMD_PAGE_MASK	(((signed long)PMD_PAGE_MASK) & __PHYSICAL_MASK)
> > > +#define PHYSICAL_PUD_PAGE_MASK	(((signed long)PUD_PAGE_MASK) & __PHYSICAL_MASK)
> > 
> > that's a really odd way of writing it, 'long' is signed by default ...
> 
> See the comment above (it was there before the patch). 'signed' can be
> considered as documentation -- we want sign-extension here.
> 
> > There seems to be 150+ such cases in the kernel source though - weird.
> > 
> > More importantly, how does this improve things on 32-bit PAE kernels? If I follow 
> > the values correctly then PMD_PAGE_MASK is 'UL' i.e. 32-bit:
> > 
> > > +#define PMD_PAGE_SIZE                (_AC(1, UL) << PMD_SHIFT)
> > > +#define PMD_PAGE_MASK                (~(PMD_PAGE_SIZE-1))
> > 
> > thus PHYSICAL_PMD_PAGE_MASK is 32-bit too:
> > 
> > > +#define PHYSICAL_PMD_PAGE_MASK       (((signed long)PMD_PAGE_MASK) & __PHYSICAL_MASK)
> > 
> > so how is the bug fixed?
> 
> Again, see the comment.

Ah, indeed! That should in fact work even better than using u64 or so, as it does 
the obvious thing for masks.

The concept will only break down once TBPAGES (well, 512 GB pages) are introduced.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/