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

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

[Kirill A. Shutemov]

Recent PAT patchset has caused issue on 32-bit PAE machines:

[    8.905943] page:eea45000 count:0 mapcount:-128 mapping:  (null) index:0x0
[    8.913041] flags: 0x40000000()
[    8.916293] page dumped because: VM_BUG_ON_PAGE(page_mapcount(page) < 0)
[    8.923204] ------------[ cut here ]------------
[    8.927958] kernel BUG at /home/build/linux-boris/mm/huge_memory.c:1485!
[    8.934860] invalid opcode: 0000 [#1] SMP
[    8.939094] Modules linked in: ahci libahci ata_generic skge r8169 firewire_ohci mii libata qla2xxx(+) scsi_transport_fc scsi_mod radeon tpm_infineon ttm backlight wmi acpi_cpufreq tpm_tis
[    8.956548] CPU: 2 PID: 1758 Comm: modprobe Not tainted 4.3.0upstream-09269-gce5c2d2 #1
[    8.964792] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014  07/18/2008
[    8.975991] task: ed84e600 ti: f6458000 task.ti: f6458000
[    8.981552] EIP: 0060:[<c11bde80>] EFLAGS: 00010246 CPU: 2
[    8.987203] EIP is at zap_huge_pmd+0x240/0x260
[    8.991778] EAX: 00000000 EBX: f6459eb0 ECX: 00000292 EDX: 00000292
[    8.998234] ESI: f6634d98 EDI: eea45000 EBP: f6459dc8 ESP: f6459d98
[    8.998355] ata1: SATA link down (SStatus 0 SControl 300)
[    9.000330] ata2: SATA link down (SStatus 0 SControl 300)
[    9.015804]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[    9.021364] CR0: 8005003b CR2: b75b21a0 CR3: 3655b880 CR4: 000006f0
[    9.027818] Stack:
[    9.029885]  00000080 00000000 80000002 ee795000 80000002 ffe00000 00000000 ffffff7f
[    9.037930]  eee6169c f70c5e40 b6600000 f6634d98 f6459e78 c119a7c8 b6600000 80000002
[    9.045972]  00000003 c18992f4 c18992f0 00000003 00000286 f6459e0c c10db5f0 00000000
[    9.054018] Call Trace:
[    9.056537]  [<c119a7c8>] unmap_single_vma+0x6e8/0x7c0
[    9.061829]  [<c10db5f0>] ? __wake_up+0x40/0x50
[    9.063587] firewire_core 0000:08:05.0: created device fw0: GUID 000000001a1a2f03, S800
[    9.074736]  [<c119a8e7>] unmap_vmas+0x47/0x80
[    9.079312]  [<c11a0c44>] unmap_region+0x74/0xc0
[    9.084067]  [<c11a2d50>] do_munmap+0x1b0/0x280
[    9.088732]  [<c11a2e58>] vm_munmap+0x38/0x50
[    9.093218]  [<c11a2e88>] SyS_munmap+0x18/0x20
[    9.097795]  [<c1003861>] do_fast_syscall_32+0xa1/0x270
[    9.103176]  [<c1095400>] ? __do_page_fault+0x430/0x430
[    9.108559]  [<c169de51>] sysenter_past_esp+0x36/0x55
[    9.113761] Code: 00 e9 05 fe ff ff 90 8d 74 26 00 0f 0b eb fe ba 4c e1 7a c1 89 f8 e8 f0 91 fd ff 0f 0b eb fe ba 6c e1 7a c1 89 f8 e8 e0 91 fd ff <0f> 0b eb fe ba c4 e1 7a c1 89 f8 e8 d0 91 fd ff 0f 0b eb fe 8d
[    9.133727] EIP: [<c11bde80>] zap_huge_pmd+0x240/0x260 SS:ESP 0068:f6459d98
[    9.140929] ---[ end trace cba8fb1fc2e2e78a ]---

The problem is in pmd_pfn_mask() and pmd_flags_mask(). These helpers use
PMD_PAGE_MASK to calculate resulting mask. PMD_PAGE_MASK is 'unsigned
long', not 'unsigned long long' as physaddr_t. As result upper bits of
resulting mask is truncated.

The patch reworks code to use PMD_SHIFT as base of mask calculation
instead of PMD_PAGE_MASK.

pud_pfn_mask() and pud_flags_mask() aren't problematic since we don't
have PUD page table level on 32-bit systems, but they reworked too to be
consistent with PMD counterpart.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reported-and-Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Fixes: f70abb0fc3da ("x86/asm: Fix pud/pmd interfaces to handle large PAT bit")
Cc: Toshi Kani <toshi.kani@hpe.com>
---
 arch/x86/include/asm/pgtable_types.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index dd5b0aa9dd2f..c1e797266ce9 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
 static inline pudval_t pud_pfn_mask(pud_t pud)
 {
 	if (native_pud_val(pud) & _PAGE_PSE)
-		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
+		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
 	else
 		return PTE_PFN_MASK;
 }
 
 static inline pudval_t pud_flags_mask(pud_t pud)
 {
-	if (native_pud_val(pud) & _PAGE_PSE)
-		return ~(PUD_PAGE_MASK & (pudval_t)PHYSICAL_PAGE_MASK);
-	else
-		return ~PTE_PFN_MASK;
+	return ~pud_pfn_mask(pud);
 }
 
 static inline pudval_t pud_flags(pud_t pud)
@@ -300,17 +297,14 @@ static inline pudval_t pud_flags(pud_t pud)
 static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
 {
 	if (native_pmd_val(pmd) & _PAGE_PSE)
-		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
+		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
 	else
 		return PTE_PFN_MASK;
 }
 
 static inline pmdval_t pmd_flags_mask(pmd_t pmd)
 {
-	if (native_pmd_val(pmd) & _PAGE_PSE)
-		return ~(PMD_PAGE_MASK & (pmdval_t)PHYSICAL_PAGE_MASK);
-	else
-		return ~PTE_PFN_MASK;
+	return ~pmd_pfn_mask(pmd);
 }
 
 static inline pmdval_t pmd_flags(pmd_t pmd)
-- 
2.6.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Toshi Kani]

On Tue, 2015-11-10 at 01:18 +0200, Kirill A. Shutemov wrote:
> Recent PAT patchset has caused issue on 32-bit PAE machines:
 :
> The problem is in pmd_pfn_mask() and pmd_flags_mask(). These helpers use
> PMD_PAGE_MASK to calculate resulting mask. PMD_PAGE_MASK is 'unsigned
> long', not 'unsigned long long' as physaddr_t. As result upper bits of
> resulting mask is truncated.
> 
> The patch reworks code to use PMD_SHIFT as base of mask calculation
> instead of PMD_PAGE_MASK.
> 
> pud_pfn_mask() and pud_flags_mask() aren't problematic since we don't
> have PUD page table level on 32-bit systems, but they reworked too to be
> consistent with PMD counterpart.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reported-and-Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Fixes: f70abb0fc3da ("x86/asm: Fix pud/pmd interfaces to handle large PAT
> bit")
> Cc: Toshi Kani <toshi.kani@hpe.com>
> ---
>  arch/x86/include/asm/pgtable_types.h | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pgtable_types.h
> b/arch/x86/include/asm/pgtable_types.h
> index dd5b0aa9dd2f..c1e797266ce9 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
>  static inline pudval_t pud_pfn_mask(pud_t pud)
>  {
>  	if (native_pud_val(pud) & _PAGE_PSE)
> -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;

Thanks for the fix!  Should we fix the PMD/PUD MASK/SIZE macros, so that we do
not hit the same issue again when they are used? 

--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -17,10 +17,10 @@
    (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_SIZE          (_AC(1, ULL) << PMD_SHIFT)
 #define PMD_PAGE_MASK          (~(PMD_PAGE_SIZE-1))

-#define PUD_PAGE_SIZE          (_AC(1, UL) << PUD_SHIFT)
+#define PUD_PAGE_SIZE          (_AC(1, ULL) << PUD_SHIFT)
 #define PUD_PAGE_MASK          (~(PUD_PAGE_SIZE-1))

Thanks,
-Toshi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Mon, Nov 09, 2015 at 04:43:11PM -0700, Toshi Kani wrote:
> On Tue, 2015-11-10 at 01:18 +0200, Kirill A. Shutemov wrote:
> > Recent PAT patchset has caused issue on 32-bit PAE machines:
>  :
> > The problem is in pmd_pfn_mask() and pmd_flags_mask(). These helpers use
> > PMD_PAGE_MASK to calculate resulting mask. PMD_PAGE_MASK is 'unsigned
> > long', not 'unsigned long long' as physaddr_t. As result upper bits of
> > resulting mask is truncated.
> > 
> > The patch reworks code to use PMD_SHIFT as base of mask calculation
> > instead of PMD_PAGE_MASK.
> > 
> > pud_pfn_mask() and pud_flags_mask() aren't problematic since we don't
> > have PUD page table level on 32-bit systems, but they reworked too to be
> > consistent with PMD counterpart.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Reported-and-Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Fixes: f70abb0fc3da ("x86/asm: Fix pud/pmd interfaces to handle large PAT
> > bit")
> > Cc: Toshi Kani <toshi.kani@hpe.com>
> > ---
> >  arch/x86/include/asm/pgtable_types.h | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/pgtable_types.h
> > b/arch/x86/include/asm/pgtable_types.h
> > index dd5b0aa9dd2f..c1e797266ce9 100644
> > --- a/arch/x86/include/asm/pgtable_types.h
> > +++ b/arch/x86/include/asm/pgtable_types.h
> > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> >  static inline pudval_t pud_pfn_mask(pud_t pud)
> >  {
> >  	if (native_pud_val(pud) & _PAGE_PSE)
> > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> 
> Thanks for the fix!  Should we fix the PMD/PUD MASK/SIZE macros, so that we do
> not hit the same issue again when they are used? 

I don't this so. PAGE_SIZE is not 'unsigned long long'. And all *PAGE_MASK
are usually applied to virtual addresses which are 'unsigned long'.
I think it's safer to leave them as they are.

> 
> --- a/arch/x86/include/asm/page_types.h
> +++ b/arch/x86/include/asm/page_types.h
> @@ -17,10 +17,10 @@
>     (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_SIZE          (_AC(1, ULL) << PMD_SHIFT)
>  #define PMD_PAGE_MASK          (~(PMD_PAGE_SIZE-1))
> 
> -#define PUD_PAGE_SIZE          (_AC(1, UL) << PUD_SHIFT)
> +#define PUD_PAGE_SIZE          (_AC(1, ULL) << PUD_SHIFT)
>  #define PUD_PAGE_MASK          (~(PUD_PAGE_SIZE-1))
> 
> Thanks,
> -Toshi
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Toshi Kani]

On Tue, 2015-11-10 at 01:57 +0200, Kirill A. Shutemov wrote:
> On Mon, Nov 09, 2015 at 04:43:11PM -0700, Toshi Kani wrote:
> > On Tue, 2015-11-10 at 01:18 +0200, Kirill A. Shutemov wrote:
> > > Recent PAT patchset has caused issue on 32-bit PAE machines:
> >  :
> > > The problem is in pmd_pfn_mask() and pmd_flags_mask(). These helpers use
> > > PMD_PAGE_MASK to calculate resulting mask. PMD_PAGE_MASK is 'unsigned
> > > long', not 'unsigned long long' as physaddr_t. As result upper bits of
> > > resulting mask is truncated.
> > > 
> > > The patch reworks code to use PMD_SHIFT as base of mask calculation
> > > instead of PMD_PAGE_MASK.
> > > 
> > > pud_pfn_mask() and pud_flags_mask() aren't problematic since we don't
> > > have PUD page table level on 32-bit systems, but they reworked too to be
> > > consistent with PMD counterpart.
> > > 
> > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Reported-and-Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > > Fixes: f70abb0fc3da ("x86/asm: Fix pud/pmd interfaces to handle large PAT
> > > bit")
> > > Cc: Toshi Kani <toshi.kani@hpe.com>
> > > ---
> > >  arch/x86/include/asm/pgtable_types.h | 14 ++++----------
> > >  1 file changed, 4 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/pgtable_types.h
> > > b/arch/x86/include/asm/pgtable_types.h
> > > index dd5b0aa9dd2f..c1e797266ce9 100644
> > > --- a/arch/x86/include/asm/pgtable_types.h
> > > +++ b/arch/x86/include/asm/pgtable_types.h
> > > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> > >  static inline pudval_t pud_pfn_mask(pud_t pud)
> > >  {
> > >  	if (native_pud_val(pud) & _PAGE_PSE)
> > > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> > 
> > Thanks for the fix!  Should we fix the PMD/PUD MASK/SIZE macros, so that we 
> > do not hit the same issue again when they are used?
> 
> I don't this so. PAGE_SIZE is not 'unsigned long long'. And all *PAGE_MASK
> are usually applied to virtual addresses which are 'unsigned long'.
> I think it's safer to leave them as they are.

Thanks for the explanation!  That makes sense.

Reviewed-by: Toshi Kani <toshi.kani@hpe.com>

-Toshi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Borislav Petkov]

On Tue, Nov 10, 2015 at 01:18:10AM +0200, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
> index dd5b0aa9dd2f..c1e797266ce9 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
>  static inline pudval_t pud_pfn_mask(pud_t pud)
>  {
>  	if (native_pud_val(pud) & _PAGE_PSE)
> -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;

In file included from ./arch/x86/include/asm/boot.h:5:0,
                 from ./arch/x86/boot/boot.h:26,
                 from arch/x86/realmode/rm/wakemain.c:2:
./arch/x86/include/asm/pgtable_types.h: In function a??pud_pfn_maska??:
./arch/x86/include/asm/pgtable_types.h:282:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
          ^
./arch/x86/include/asm/pgtable_types.h: In function a??pmd_pfn_maska??:
./arch/x86/include/asm/pgtable_types.h:300:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
          ^
In file included from ./arch/x86/include/asm/boot.h:5:0,
                 from arch/x86/realmode/rm/../../boot/boot.h:26,
                 from arch/x86/realmode/rm/../../boot/video-mode.c:18,
                 from arch/x86/realmode/rm/video-mode.c:1:
./arch/x86/include/asm/pgtable_types.h: In function a??pud_pfn_maska??:
./arch/x86/include/asm/pgtable_types.h:282:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
   return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
          ^
...

That's a 64-bit config.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Tue, Nov 10, 2015 at 01:34:29PM +0100, Borislav Petkov wrote:
> On Tue, Nov 10, 2015 at 01:18:10AM +0200, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
> > index dd5b0aa9dd2f..c1e797266ce9 100644
> > --- a/arch/x86/include/asm/pgtable_types.h
> > +++ b/arch/x86/include/asm/pgtable_types.h
> > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> >  static inline pudval_t pud_pfn_mask(pud_t pud)
> >  {
> >  	if (native_pud_val(pud) & _PAGE_PSE)
> > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> 
> In file included from ./arch/x86/include/asm/boot.h:5:0,
>                  from ./arch/x86/boot/boot.h:26,
>                  from arch/x86/realmode/rm/wakemain.c:2:
> ./arch/x86/include/asm/pgtable_types.h: In function a??pud_pfn_maska??:
> ./arch/x86/include/asm/pgtable_types.h:282:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>    return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
>           ^
> ./arch/x86/include/asm/pgtable_types.h: In function a??pmd_pfn_maska??:
> ./arch/x86/include/asm/pgtable_types.h:300:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>    return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
>           ^
> In file included from ./arch/x86/include/asm/boot.h:5:0,
>                  from arch/x86/realmode/rm/../../boot/boot.h:26,
>                  from arch/x86/realmode/rm/../../boot/video-mode.c:18,
>                  from arch/x86/realmode/rm/video-mode.c:1:
> ./arch/x86/include/asm/pgtable_types.h: In function a??pud_pfn_maska??:
> ./arch/x86/include/asm/pgtable_types.h:282:10: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>    return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
>           ^
> ...
> 
> That's a 64-bit config.

Oh.. pmdval_t/pudval_t is 'unsinged long' on 64 bit. But realmode code
uses -m16 which makes 'unsigned long' 32-bit therefore truncation warning.

These helpers not really used in realmode code. I see few ways out:

 - convert helpers to macros to avoid their translation;

 - wrap the code into not-for-realmode ifdef. (Do we have any?);

 - convert pudval_t/pmdval_t to u64 for 64-bit. I'm not sure what side
   effects would it have.

Any opinions?

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Borislav Petkov]

On Tue, Nov 10, 2015 at 03:53:03PM +0200, Kirill A. Shutemov wrote:
> Oh.. pmdval_t/pudval_t is 'unsinged long' on 64 bit. But realmode code
> uses -m16 which makes 'unsigned long' 32-bit therefore truncation warning.
> 
> These helpers not really used in realmode code.

Hrrm, yeah, that's just the nasty include hell causing it. The diff
below fixes it with my config but it'll probably need a more careful
analysis and reshuffling of includes/defines.

Certainly better to do that than accomodating realmode to not throw
warnings with ifdeffery...

---
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 0033e96c3f09..9011a88353de 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -23,7 +23,6 @@
 #include <stdarg.h>
 #include <linux/types.h>
 #include <linux/edd.h>
-#include <asm/boot.h>
 #include <asm/setup.h>
 #include "bitops.h"
 #include "ctype.h"
diff --git a/arch/x86/boot/video-mode.c b/arch/x86/boot/video-mode.c
index aa8a96b052e3..896077ed3381 100644
--- a/arch/x86/boot/video-mode.c
+++ b/arch/x86/boot/video-mode.c
@@ -19,6 +19,9 @@
 #include "video.h"
 #include "vesa.h"
 
+#define NORMAL_VGA	0xffff		/* 80x25 mode */
+#define EXTENDED_VGA	0xfffe		/* 80x50 mode */
+
 /*
  * Common variables
  */
diff --git a/arch/x86/boot/video.c b/arch/x86/boot/video.c
index 05111bb8d018..a839448038b6 100644
--- a/arch/x86/boot/video.c
+++ b/arch/x86/boot/video.c
@@ -17,6 +17,8 @@
 #include "video.h"
 #include "vesa.h"
 
+#define ASK_VGA		0xfffd		/* ask for it at bootup */
+
 static u16 video_segment;
 
 static void store_cursor_position(void)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 48d34d28f5a6..cd0fc0cc78bc 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,7 +1,6 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
-#include <asm/pgtable_types.h>
 #include <asm/bootparam.h>
 
 struct mpc_bus;

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Tue, Nov 10, 2015 at 03:46:48PM +0100, Borislav Petkov wrote:
> On Tue, Nov 10, 2015 at 03:53:03PM +0200, Kirill A. Shutemov wrote:
> > Oh.. pmdval_t/pudval_t is 'unsinged long' on 64 bit. But realmode code
> > uses -m16 which makes 'unsigned long' 32-bit therefore truncation warning.
> > 
> > These helpers not really used in realmode code.
> 
> Hrrm, yeah, that's just the nasty include hell causing it. The diff
> below fixes it with my config but it'll probably need a more careful
> analysis and reshuffling of includes/defines.
> 
> Certainly better to do that than accomodating realmode to not throw
> warnings with ifdeffery...

Yeah. Looks good to me.

> ---
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 0033e96c3f09..9011a88353de 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -23,7 +23,6 @@
>  #include <stdarg.h>
>  #include <linux/types.h>
>  #include <linux/edd.h>
> -#include <asm/boot.h>
>  #include <asm/setup.h>
>  #include "bitops.h"
>  #include "ctype.h"
> diff --git a/arch/x86/boot/video-mode.c b/arch/x86/boot/video-mode.c
> index aa8a96b052e3..896077ed3381 100644
> --- a/arch/x86/boot/video-mode.c
> +++ b/arch/x86/boot/video-mode.c
> @@ -19,6 +19,9 @@
>  #include "video.h"
>  #include "vesa.h"
>  
> +#define NORMAL_VGA	0xffff		/* 80x25 mode */
> +#define EXTENDED_VGA	0xfffe		/* 80x50 mode */
> +
>  /*
>   * Common variables
>   */
> diff --git a/arch/x86/boot/video.c b/arch/x86/boot/video.c
> index 05111bb8d018..a839448038b6 100644
> --- a/arch/x86/boot/video.c
> +++ b/arch/x86/boot/video.c
> @@ -17,6 +17,8 @@
>  #include "video.h"
>  #include "vesa.h"
>  
> +#define ASK_VGA		0xfffd		/* ask for it at bootup */
> +
>  static u16 video_segment;
>  
>  static void store_cursor_position(void)
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 48d34d28f5a6..cd0fc0cc78bc 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -1,7 +1,6 @@
>  #ifndef _ASM_X86_PLATFORM_H
>  #define _ASM_X86_PLATFORM_H
>  
> -#include <asm/pgtable_types.h>
>  #include <asm/bootparam.h>
>  
>  struct mpc_bus;
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> ECO tip #101: Trim your mails when you reply.

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Borislav Petkov]

On Tue, Nov 10, 2015 at 05:07:13PM +0200, Kirill A. Shutemov wrote:
> Yeah. Looks good to me.

It gets even cleaner. Let me run it through the *config build tests.

---
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 0033e96c3f09..9011a88353de 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -23,7 +23,6 @@
 #include <stdarg.h>
 #include <linux/types.h>
 #include <linux/edd.h>
-#include <asm/boot.h>
 #include <asm/setup.h>
 #include "bitops.h"
 #include "ctype.h"
diff --git a/arch/x86/boot/video-mode.c b/arch/x86/boot/video-mode.c
index aa8a96b052e3..95c7a818c0ed 100644
--- a/arch/x86/boot/video-mode.c
+++ b/arch/x86/boot/video-mode.c
@@ -19,6 +19,8 @@
 #include "video.h"
 #include "vesa.h"
 
+#include <uapi/asm/boot.h>
+
 /*
  * Common variables
  */
diff --git a/arch/x86/boot/video.c b/arch/x86/boot/video.c
index 05111bb8d018..77780e386e9b 100644
--- a/arch/x86/boot/video.c
+++ b/arch/x86/boot/video.c
@@ -13,6 +13,8 @@
  * Select video mode
  */
 
+#include <uapi/asm/boot.h>
+
 #include "boot.h"
 #include "video.h"
 #include "vesa.h"
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 48d34d28f5a6..cd0fc0cc78bc 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,7 +1,6 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
-#include <asm/pgtable_types.h>
 #include <asm/bootparam.h>
 
 struct mpc_bus;

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Borislav Petkov]

On Tue, Nov 10, 2015 at 06:04:47PM +0100, Borislav Petkov wrote:
> On Tue, Nov 10, 2015 at 05:07:13PM +0200, Kirill A. Shutemov wrote:
> > Yeah. Looks good to me.
> 
> It gets even cleaner. Let me run it through the *config build tests.

Hohumm, it passes. Here's what I ended up committing:

---
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Tue, 10 Nov 2015 01:18:10 +0200
Subject: [PATCH] x86/mm: Fix regression with huge pages on PAE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Recent PAT patchset has caused issue on 32-bit PAE machines:

page:eea45000 count:0 mapcount:-128 mapping:  (null) index:0x0
flags: 0x40000000()
page dumped because: VM_BUG_ON_PAGE(page_mapcount(page) < 0)
------------[ cut here ]------------
kernel BUG at /home/build/linux-boris/mm/huge_memory.c:1485!
invalid opcode: 0000 [#1] SMP
Modules linked in: ahci libahci ata_generic skge r8169 firewire_ohci mii libata qla2xxx(+) scsi_transport_fc scsi_mod radeon tpm_infineon ttm backlight wmi acpi_cpufreq tpm_tis
CPU: 2 PID: 1758 Comm: modprobe Not tainted 4.3.0upstream-09269-gce5c2d2 #1
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014  07/18/2008
task: ed84e600 ti: f6458000 task.ti: f6458000
EIP: 0060:[<c11bde80>] EFLAGS: 00010246 CPU: 2
EIP is at zap_huge_pmd+0x240/0x260
EAX: 00000000 EBX: f6459eb0 ECX: 00000292 EDX: 00000292
ESI: f6634d98 EDI: eea45000 EBP: f6459dc8 ESP: f6459d98
ata1: SATA link down (SStatus 0 SControl 300)
ata2: SATA link down (SStatus 0 SControl 300)
 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
CR0: 8005003b CR2: b75b21a0 CR3: 3655b880 CR4: 000006f0
Stack:
 ...
Call Trace:
 unmap_single_vma
 ? __wake_up
 unmap_vmas
 unmap_region
 do_munmap
 vm_munmap
 SyS_munmap
 do_fast_syscall_32
 ? __do_page_fault
 sysenter_past_esp
Code: 00 e9 05 fe ff ff 90 8d 74 26 00 0f 0b eb fe ba 4c e1 7a c1 89 f8 e8 f0 91 fd ff 0f 0b eb fe ba 6c e1 7a c1 89 f8 e8 e0 91 fd ff <0f> 0b eb fe ba c4 e1 7a c1 89 f8 e8 d0 91 fd ff 0f 0b eb fe 8d
EIP: [<c11bde80>] zap_huge_pmd+0x240/0x260 SS:ESP 0068:f6459d98
---[ end trace cba8fb1fc2e2e78a ]---

The problem is in pmd_pfn_mask() and pmd_flags_mask(). These helpers use
PMD_PAGE_MASK to calculate resulting mask. PMD_PAGE_MASK is 'unsigned
long', not 'unsigned long long' as phys_addr_t. As result upper bits of
resulting mask is truncated.

The patch reworks code to use PMD_SHIFT as base of mask calculation
instead of PMD_PAGE_MASK.

pud_pfn_mask() and pud_flags_mask() aren't problematic since we don't
have PUD page table level on 32-bit systems, but they are reworked too
to be consistent with PMD counterpart.

Reported-and-Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: elliott@hpe.com
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: JA 1/4 rgen Gross <jgross@suse.com>
Cc: konrad.wilk@oracle.com
Cc: linux-mm <linux-mm@kvack.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: f70abb0fc3da ("x86/asm: Fix pud/pmd interfaces to handle large PAT bit")
Link: http://lkml.kernel.org/r/1447111090-8526-1-git-send-email-kirill.shutemov@linux.intel.com
[ Fix -Woverflow warnings from the realmode code. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/boot/boot.h                 |  1 -
 arch/x86/boot/video-mode.c           |  2 ++
 arch/x86/boot/video.c                |  2 ++
 arch/x86/include/asm/pgtable_types.h | 14 ++++----------
 arch/x86/include/asm/x86_init.h      |  1 -
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 0033e96c3f09..9011a88353de 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -23,7 +23,6 @@
 #include <stdarg.h>
 #include <linux/types.h>
 #include <linux/edd.h>
-#include <asm/boot.h>
 #include <asm/setup.h>
 #include "bitops.h"
 #include "ctype.h"
diff --git a/arch/x86/boot/video-mode.c b/arch/x86/boot/video-mode.c
index aa8a96b052e3..95c7a818c0ed 100644
--- a/arch/x86/boot/video-mode.c
+++ b/arch/x86/boot/video-mode.c
@@ -19,6 +19,8 @@
 #include "video.h"
 #include "vesa.h"
 
+#include <uapi/asm/boot.h>
+
 /*
  * Common variables
  */
diff --git a/arch/x86/boot/video.c b/arch/x86/boot/video.c
index 05111bb8d018..77780e386e9b 100644
--- a/arch/x86/boot/video.c
+++ b/arch/x86/boot/video.c
@@ -13,6 +13,8 @@
  * Select video mode
  */
 
+#include <uapi/asm/boot.h>
+
 #include "boot.h"
 #include "video.h"
 #include "vesa.h"
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index dd5b0aa9dd2f..c1e797266ce9 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
 static inline pudval_t pud_pfn_mask(pud_t pud)
 {
 	if (native_pud_val(pud) & _PAGE_PSE)
-		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
+		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
 	else
 		return PTE_PFN_MASK;
 }
 
 static inline pudval_t pud_flags_mask(pud_t pud)
 {
-	if (native_pud_val(pud) & _PAGE_PSE)
-		return ~(PUD_PAGE_MASK & (pudval_t)PHYSICAL_PAGE_MASK);
-	else
-		return ~PTE_PFN_MASK;
+	return ~pud_pfn_mask(pud);
 }
 
 static inline pudval_t pud_flags(pud_t pud)
@@ -300,17 +297,14 @@ static inline pudval_t pud_flags(pud_t pud)
 static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
 {
 	if (native_pmd_val(pmd) & _PAGE_PSE)
-		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
+		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
 	else
 		return PTE_PFN_MASK;
 }
 
 static inline pmdval_t pmd_flags_mask(pmd_t pmd)
 {
-	if (native_pmd_val(pmd) & _PAGE_PSE)
-		return ~(PMD_PAGE_MASK & (pmdval_t)PHYSICAL_PAGE_MASK);
-	else
-		return ~PTE_PFN_MASK;
+	return ~pmd_pfn_mask(pmd);
 }
 
 static inline pmdval_t pmd_flags(pmd_t pmd)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 48d34d28f5a6..cd0fc0cc78bc 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,7 +1,6 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
-#include <asm/pgtable_types.h>
 #include <asm/bootparam.h>
 
 struct mpc_bus;
-- 
2.3.5

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
>  static inline pudval_t pud_pfn_mask(pud_t pud)
>  {
>  	if (native_pud_val(pud) & _PAGE_PSE)
> -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
>  	else
>  		return PTE_PFN_MASK;
>  }

>  static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
>  {
>  	if (native_pmd_val(pmd) & _PAGE_PSE)
> -		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> +		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
>  	else
>  		return PTE_PFN_MASK;
>  }

So instead of uglifying the code, why not fix the real bug: change the 
PMD_PAGE_MASK/PUD_PAGE_MASK definitions to be 64-bit everywhere?

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Thu, Nov 12, 2015 at 08:48:54AM +0100, Ingo Molnar wrote:
> 
> * Borislav Petkov <bp@alien8.de> wrote:
> 
> > --- a/arch/x86/include/asm/pgtable_types.h
> > +++ b/arch/x86/include/asm/pgtable_types.h
> > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> >  static inline pudval_t pud_pfn_mask(pud_t pud)
> >  {
> >  	if (native_pud_val(pud) & _PAGE_PSE)
> > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> >  	else
> >  		return PTE_PFN_MASK;
> >  }
> 
> >  static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
> >  {
> >  	if (native_pmd_val(pmd) & _PAGE_PSE)
> > -		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > +		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> >  	else
> >  		return PTE_PFN_MASK;
> >  }
> 
> So instead of uglifying the code, why not fix the real bug: change the 
> PMD_PAGE_MASK/PUD_PAGE_MASK definitions to be 64-bit everywhere?

*PAGE_MASK are usually applied to virtual addresses. I don't think it
should anything but 'unsigned long'. This is odd use case really.

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> On Thu, Nov 12, 2015 at 08:48:54AM +0100, Ingo Molnar wrote:
> > 
> > * Borislav Petkov <bp@alien8.de> wrote:
> > 
> > > --- a/arch/x86/include/asm/pgtable_types.h
> > > +++ b/arch/x86/include/asm/pgtable_types.h
> > > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> > >  static inline pudval_t pud_pfn_mask(pud_t pud)
> > >  {
> > >  	if (native_pud_val(pud) & _PAGE_PSE)
> > > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> > >  	else
> > >  		return PTE_PFN_MASK;
> > >  }
> > 
> > >  static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
> > >  {
> > >  	if (native_pmd_val(pmd) & _PAGE_PSE)
> > > -		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > > +		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> > >  	else
> > >  		return PTE_PFN_MASK;
> > >  }
> > 
> > So instead of uglifying the code, why not fix the real bug: change the 
> > PMD_PAGE_MASK/PUD_PAGE_MASK definitions to be 64-bit everywhere?
> 
> *PAGE_MASK are usually applied to virtual addresses. I don't think it
> should anything but 'unsigned long'. This is odd use case really.

So we already have PHYSICAL_PAGE_MASK, why not introduce PHYSICAL_PMD_MASK et al, 
instead of uglifying the code?

But, what problems do you expect with having a wider mask than its primary usage? 
If it's used for 32-bit values it will be truncated down safely. (But I have not 
tested it, so I might be missing some complication.)

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> 
> > On Thu, Nov 12, 2015 at 08:48:54AM +0100, Ingo Molnar wrote:
> > > 
> > > * Borislav Petkov <bp@alien8.de> wrote:
> > > 
> > > > --- a/arch/x86/include/asm/pgtable_types.h
> > > > +++ b/arch/x86/include/asm/pgtable_types.h
> > > > @@ -279,17 +279,14 @@ static inline pmdval_t native_pmd_val(pmd_t pmd)
> > > >  static inline pudval_t pud_pfn_mask(pud_t pud)
> > > >  {
> > > >  	if (native_pud_val(pud) & _PAGE_PSE)
> > > > -		return PUD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > > > +		return ~((1ULL << PUD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> > > >  	else
> > > >  		return PTE_PFN_MASK;
> > > >  }
> > > 
> > > >  static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
> > > >  {
> > > >  	if (native_pmd_val(pmd) & _PAGE_PSE)
> > > > -		return PMD_PAGE_MASK & PHYSICAL_PAGE_MASK;
> > > > +		return ~((1ULL << PMD_SHIFT) - 1) & PHYSICAL_PAGE_MASK;
> > > >  	else
> > > >  		return PTE_PFN_MASK;
> > > >  }
> > > 
> > > So instead of uglifying the code, why not fix the real bug: change the 
> > > PMD_PAGE_MASK/PUD_PAGE_MASK definitions to be 64-bit everywhere?
> > 
> > *PAGE_MASK are usually applied to virtual addresses. I don't think it
> > should anything but 'unsigned long'. This is odd use case really.
> 
> So we already have PHYSICAL_PAGE_MASK, why not introduce PHYSICAL_PMD_MASK et al, 
> instead of uglifying the code?

Okay, makes sense. Check out the patch below.

> But, what problems do you expect with having a wider mask than its primary usage? 
> If it's used for 32-bit values it will be truncated down safely. (But I have not 
> tested it, so I might be missing some complication.)

Yeah, I basically worry about non realized side effect.

And these masks are defined via {PMD,PUD}_PAGE_SIZE. Should we change them
to 'unsigned long long' too or leave alone? What about PAGE_SIZE and
PAGE_MASK? Need to be converted too?

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

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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 ...

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?

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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.
I've checked that it generates correct value (using kernel/bounds.c).

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
> > 
> > * Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Boris Ostrovsky]

On 11/12/2015 04:00 AM, Kirill A. Shutemov wrote:
> On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
>> * Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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


Kirill, where are we with this patch?

-boris

>>> @@ -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.
> I've checked that it generates correct value (using kernel/bounds.c).
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kirill A. Shutemov]

On Tue, Nov 24, 2015 at 09:59:27AM -0500, Boris Ostrovsky wrote:
> On 11/12/2015 04:00 AM, Kirill A. Shutemov wrote:
> >On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
> >>* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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
> 
> 
> Kirill, where are we with this patch?

I haven't seen any actionable objections to the updated patch.
Not sure why it's not applied.

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Borislav Petkov]

On Tue, Nov 24, 2015 at 10:14:49PM +0200, Kirill A. Shutemov wrote:
> I haven't seen any actionable objections to the updated patch.
> Not sure why it's not applied.

It is now.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> On Tue, Nov 24, 2015 at 09:59:27AM -0500, Boris Ostrovsky wrote:
> > On 11/12/2015 04:00 AM, Kirill A. Shutemov wrote:
> > >On Thu, Nov 12, 2015 at 09:54:18AM +0100, Ingo Molnar wrote:
> > >>* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> 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
> > 
> > 
> > Kirill, where are we with this patch?
> 
> I haven't seen any actionable objections to the updated patch.
> Not sure why it's not applied.

So I think that happened because you did not change the subject line to a new, 
fresh one, that indicates it's a patch intended to be applied.

Patches sent inside existing discussions, under the same subject, tend to be 
test-only or discussion-only patches, to be submitted for real, in 95% of the 
cases.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> > But, what problems do you expect with having a wider mask than its primary usage? 
> > If it's used for 32-bit values it will be truncated down safely. (But I have not 
> > tested it, so I might be missing some complication.)
> 
> Yeah, I basically worry about non realized side effect.

Such a worry is prudent, the best way would be to double check a disassembly of a 
before/after vmlinux and see in which functions they differ and why.

We might have similar bugs elsewhere - silently fixed by such a change.

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Linus Torvalds]

On Thu, Nov 12, 2015 at 12:00 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> So we already have PHYSICAL_PAGE_MASK, why not introduce PHYSICAL_PMD_MASK et al,
> instead of uglifying the code?

I think that's the right thing here.

> But, what problems do you expect with having a wider mask than its primary usage?
> If it's used for 32-bit values it will be truncated down safely. (But I have not
> tested it, so I might be missing some complication.)

No, it will not necessarily be truncated down. If we were to make the
regular PAGE_MASK etc that are normally used for virtual addresses be
"ull", it might easily make some calcyulations be done in 64 bits
instead. Sure, they'll probably be truncated down *eventually* when
you actually store them to some 32-bit thing, but I'd worry about it.

An example of a situation where over-long types cause problems is
simply in variadic functions (typically printk, but they do happen in
other places). Writing

    printk("page offset = %ul\n", address & PAGE_MASK);

makes sense. In the VM, addresses really are "unsigned long". But just
imagine how wrong the above goes if PAGE_MASK was made "ull".

So no, widening masks to the maximal possible type is not the answer.
They need to be the natural size.

Another possibility would be to simply make masks be _signed_ longs.
That can has its own set of problems, but it does mean that when the
mask has high bits set and it gets expanded to a wider type, the
normal C rules just do the RightThing(tm).

We've occasionally done that very explicitly. Just see how
PHYSICAL_PAGE_MASK is defined in terms of a signed PAGE_MASK.

I have this dim memory of us playing around with just making PAGE_SIZE
(and thus PAGE_MASK) always be signed, but that it caused other
problems. Signed types have downsides too.

                   Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Dan Williams]

On Thu, Nov 12, 2015 at 11:29 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Nov 12, 2015 at 12:00 AM, Ingo Molnar <mingo@kernel.org> wrote:
[..]
> I have this dim memory of us playing around with just making PAGE_SIZE
> (and thus PAGE_MASK) always be signed, but that it caused other
> problems. Signed types have downsides too.

FWIW, I ran into this recently with the pfn_t patch.  mips and powerpc
have PAGE_MASK as a signed int.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>