arm64: mm: Add pgd_page to support RCU fast_gup
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
This patch adds pgd_page definition in order to keep supporting HAVE_GENERIC_RCU_GUP configuration. In addition, it changes pud_page expression to align with pmd_page for readability.
An introduction of pgd_page resolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head = pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head = pgd_page(orig);
--- arch/arm64/include/asm/pgtable.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/arm64/include/asm/pgtable.h +3 -1
@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) return (pmd_t *)pud_page_vaddr(*pud) + pmd_index(addr); } -#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))#endif /* CONFIG_ARM64_PGTABLE_LEVELS > 2 */
@@ -437,6 +437,8 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr) return (pud_t *)pgd_page_vaddr(*pgd) + pud_index(addr); } +#define pgd_page (pgd) pfn_to_page(__phys_to_pfn(pgd_val(pgd) & PHYS_MASK)) +#endif /* CONFIG_ARM64_PGTABLE_LEVELS > 3 */ #define pgd_ERROR (pgd) __pgd_error(__FILE__, __LINE__, pgd_val(pgd))
Signature · 1 line
1.9.1
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 27 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>--- arch/arm64/include/asm/pgtable.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 5 lines
arch/arm64/include/asm/pgtable.h +1
-1@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); } -#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))
It would be cleaner to use pud_pfn here, otherwise we end up passing a physical address with the lower attributes present to __phys_to_pfn, which "knows" to shift them away.
arch/arm64/include/asm/pgtable.h static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr) · line 440
Quoted 7 lines
arch/arm64/include/asm/pgtable.h +1
-0#endif /* CONFIG_ARM64_PGTABLE_LEVELS > 2 */ @@ -437,6 +437,8 @@ static inline pud_t *pud_offset(pgd_t *pgd, unsigned long addr)return (pud_t *)pgd_page_vaddr (*pgd) +pud_index (addr); } +#define pgd_page (pgd) pfn_to_page(__phys_to_pfn(pgd_val(pgd) & PHYS_MASK))
Then you'd need to define pgd_pfn for this guy.
With that change:
Will
Catalin Marinas
maintainerPATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>--- arch/arm64/include/asm/pgtable.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }
Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.
It looks like we did the same with pmd_page() but not with pte_page(), the latter using pte_pfn().
It's not a problem with lower attributes because they are within the first 12 bits, so the shifting gets rid of them. For pmd/pud/pgd, bits above 12 and up to the actual address must be 0.
I agree with changing all of them to use pte/pmd/pud/pgd_pfn but I'm inclined to merge this patch now to fix the kernel build and we'll look at the clean-up after Christmas.
Signature · 1 line
Catalin
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 42 lines · Catalin Marinas
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>---
arch/arm64/include/asm/pgtable.h| 4 +++-1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.It looks like we did the same with
pmd_page()but not withpte_page(), the latter usingpte_pfn().
Quoted 7 lines
It's not a problem with lower attributes because they are within the first 12 bits, so the shifting gets rid of them. For pmd/pud/pgd, bits above 12 and up to the actual address must be 0.
I agree with changing all of them to use pte/pmd/pud/pgd_pfn but I'm inclined to merge this patch now to fix the kernel build and we'll look at the clean-up after Christmas.
Either way, I'm fine.
Best Regards
Jungseok Lee
Catalin Marinas
maintainerPATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 47 lines · Jungseok Lee
Quoted 46 lines · Catalin Marinas
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>---
arch/arm64/include/asm/pgtable.h| 4 +++-1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.It looks like we did the same with
pmd_page()but not withpte_page(), the latter usingpte_pfn().It's not a problem with lower attributes because they are within the first 12 bits, so the shifting gets rid of them. For pmd/pud/pgd, bits above 12 and up to the actual address must be 0.
Quoted 5 lines
Quoted 3 lines
I agree with changing all of them to use pte/pmd/pud/pgd_pfn but I'm inclined to merge this patch now to fix the kernel build and we'll look at the clean-up after Christmas.
Either way, I'm fine.
Unless you post an updated patch quickly enough ;)
Signature · 1 line
Catalin
Catalin Marinas
maintainerPATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 48 lines · Catalin Marinas
Quoted 47 lines · Jungseok Lee
Quoted 46 lines · Catalin Marinas
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>---
arch/arm64/include/asm/pgtable.h| 4 +++-1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.It looks like we did the same with
pmd_page()but not withpte_page(), the latter usingpte_pfn().It's not a problem with lower attributes because they are within the first 12 bits, so the shifting gets rid of them. For pmd/pud/pgd, bits above 12 and up to the actual address must be 0.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
I agree with changing all of them to use pte/pmd/pud/pgd_pfn but I'm inclined to merge this patch now to fix the kernel build and we'll look at the clean-up after Christmas.
Either way, I'm fine.
Unless you post an updated patch quickly enough ;)
Actually, don't worry, I'll fold Will's suggestions into your patch. I found another problem with pmd_page being defined twice.
Thanks.
Signature · 1 line
Catalin
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 53 lines · Catalin Marinas
Quoted 52 lines · Catalin Marinas
Quoted 51 lines · Jungseok Lee
Quoted 49 lines · Catalin Marinas
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration] head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>---
arch/arm64/include/asm/pgtable.h| 4 +++-1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.It looks like we did the same with
pmd_page()but not withpte_page(), the latter usingpte_pfn().It's not a problem with lower attributes because they are within the first 12 bits, so the shifting gets rid of them. For pmd/pud/pgd, bits above 12 and up to the actual address must be 0.
I agree with changing all of them to use pte/pmd/pud/pgd_pfn but I'm inclined to merge this patch now to fix the kernel build and we'll look at the clean-up after Christmas.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Either way, I'm fine.
Unless you post an updated patch quickly enough ;)
Actually, don't worry, I'll fold Will's suggestions into your patch. I found another problem with
pmd_pagebeing defined twice.
Yeah. One of them should be removed.
I tried to rework the patch with Will's suggestion, but I've failed to figure out a full story of pmd_pfn and pmd_page.
It looks like that pmd_page cannot be written using pmd_pfn unless pmd_pfn is changed since pmd_pfn masks an input value with PMD_MASK, but pmd_page does not. Additionally, I'm not sure about whether pmd_pfn with PMD_MASK works well for an user process page table.
So, my change on pmd_pfn and pmd_page is as follows. Please correct me if I am wrong.
-----8<-----
arch/arm64/include/asm/pgtable.h +2 -2
@@ -294,11 +294,11 @@ void pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address,#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT)) -#define pmd_pfn (pmd) (((pmd_val(pmd) & PMD_MASK) & PHYS_MASK) >> PAGE_SHIFT) +#define pmd_pfn (pmd) ((pmd_val(pmd) & PHYS_MASK) >> PAGE_SHIFT)#define pfn_pmd (pfn,prot) (__pmd(((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot)))#define mk_pmd (page,prot) pfn_pmd(page_to_pfn(page),prot) -#define pmd_page (pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK)) +#define pmd_page (pmd) pfn_to_page(pmd_pfn(pmd))#define pud_write (pud) pte_write(pud_pte(pud))#define pud_pfn (pud) (((pud_val(pud) & PUD_MASK) & PHYS_MASK) >> PAGE_SHIFT)
Thanks for a kind response, Catalin and Will!
Best Regards
Jungseok Lee
Catalin Marinas
maintainerPATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 14 lines · Jungseok Lee
It looks like that
pmd_pagecannot be written usingpmd_pfnunlesspmd_pfnis changed sincepmd_pfnmasks an input value withPMD_MASK, butpmd_pagedoes not. Additionally, I'm not sure about whetherpmd_pfnwithPMD_MASKworks well for an user process page table.So, my change on
pmd_pfnandpmd_pageis as follows. Please correct me if I am wrong.-----8<-----
arch/arm64/include/asm/pgtable.h +0
-0@@ -294,11 +294,11 @@ void pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address,#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
arch/arm64/include/asm/pgtable.h void pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address, · line 303
Quoted 10 lines
arch/arm64/include/asm/pgtable.h +2
-2-#define pmd_pfn (pmd) (((pmd_val(pmd) & PMD_MASK) & PHYS_MASK) >> PAGE_SHIFT) +#define pmd_pfn (pmd) ((pmd_val(pmd) & PHYS_MASK) >> PAGE_SHIFT)#define pfn_pmd (pfn,prot) (__pmd(((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot)))#define mk_pmd (page,prot) pfn_pmd(page_to_pfn(page),prot) -#define pmd_page (pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK)) +#define pmd_page (pmd) pfn_to_page(pmd_pfn(pmd))#define pud_write (pud) pte_write(pud_pte(pud))#define pud_pfn (pud) (((pud_val(pud) & PUD_MASK) & PHYS_MASK) >> PAGE_SHIFT)
It looks like pmd_pfn() is only used for huge pages, so the original code was safe. As I said, I won't do further changes here, at least not for 3.19.
Signature · 1 line
Catalin
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 21 lines · Catalin Marinas
Quoted 20 lines · Jungseok Lee
It looks like that
pmd_pagecannot be written usingpmd_pfnunlesspmd_pfnis changed sincepmd_pfnmasks an input value withPMD_MASK, butpmd_pagedoes not. Additionally, I'm not sure about whetherpmd_pfnwithPMD_MASKworks well for an user process page table.So, my change on
pmd_pfnandpmd_pageis as follows. Please correct me if I am wrong.-----8<-----
arch/arm64/include/asm/pgtable.h +1
-1@@ -294,11 +294,11 @@ void pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address,#define pmd_mkhuge (pmd) (__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT)) -#define pmd_pfn (pmd) (((pmd_val(pmd) & PMD_MASK) & PHYS_MASK) >> PAGE_SHIFT) +#define pmd_pfn (pmd) ((pmd_val(pmd) & PHYS_MASK) >> PAGE_SHIFT)#define pfn_pmd (pfn,prot) (__pmd(((phys_addr_t)(pfn) << PAGE_SHIFT) | pgprot_val(prot)))#define mk_pmd (page,prot) pfn_pmd(page_to_pfn(page),prot)
Quoted 8 lines
arch/arm64/include/asm/pgtable.h void pmdp_splitting_flush(struct vm_area_struct *vma, unsigned long address, · line 303
Quoted 4 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pmd_page (pmd) pfn_to_page(__phys_to_pfn(pmd_val(pmd) & PHYS_MASK)) +#define pmd_page (pmd) pfn_to_page(pmd_pfn(pmd))#define pud_write (pud) pte_write(pud_pte(pud))#define pud_pfn (pud) (((pud_val(pud) & PUD_MASK) & PHYS_MASK) >> PAGE_SHIFT)It looks like
pmd_pfn()is only used for huge pages, so the original code was safe. As I said, I won't do further changes here, at least not for 3.19.
Okay, everything is clear.
Best Regards
Jungseok Lee
Catalin Marinas
maintainerPATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
In reply to Will DeaconQuoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>--- arch/arm64/include/asm/pgtable.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }
Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.
OK, I tried this, together with aligning pmd_page to use pmd_pfn, and after debugging I realised why it is a bad idea. I had a feeling that I tried this before but didn't remember the details.
I'll take the pmd_page() example, it is used in two scenarios:
a) getting the first page structure of a huge page
b) getting the page structure of the pte page pointed at by the pmd
In the first case, we know that the pmd value is aligned to PMD_SIZE as it points to the physical address of a huge page, so pmd_pfn() masking is fine. In the second case, such masking is not, a pte page can be just PAGE_SIZE aligned. So using pmd_pfn to mask out the bits above 12 in the pmd val breaks case (b) above.
While we don't have such use-case (b) for pud_page(), I would rather keep them consistent with pmd_page.
A better fix would be to change such pmd_page() usage to pmd_pgtable() in the core mm code and have a different implementation for the latter. I guess we can leave this exercise to Steve ;).
In the meantime, I'll merge Jungseok's original patch.
Signature · 1 line
Catalin
PATCH 1/1 arm64: mm: Add pgd_page to support RCU fast_gup
Quoted 54 lines · Catalin Marinas
Quoted 31 lines · Will Deacon
Quoted 30 lines · Jungseok Lee
This patch adds
pgd_pagedefinition in order to keep supportingHAVE_GENERIC_RCU_GUPconfiguration. In addition, it changespud_pageexpression to align withpmd_pagefor readability.An introduction of
pgd_pageresolves the following build breakage under 4KB + 4Level memory management combo.
mm/gup.c: In function 'gup_huge_pgd':mm/gup.c:889:2: error: implicit declaration of function 'pgd_page' [-Werror=implicit-function-declaration]
head =pgd_page(orig);
^mm/gup.c:889:7: warning: assignment makes pointer from integer without a cast
head =pgd_page(orig);Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Steve Capper <[email protected]>Signed-off-by Jungseok Lee <[email protected]>---
arch/arm64/include/asm/pgtable.h| 4 +++-1 file changed, 3 insertions(+), 1 deletion(-)arch/arm64/include/asm/pgtable.h +0
-0@@ -401,7 +401,7 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)return (pmd_t *)pud_page_vaddr (*pud) +pmd_index (addr); }Quoted 6 lines
arch/arm64/include/asm/pgtable.h static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr) · line 404
Quoted 2 lines
arch/arm64/include/asm/pgtable.h +1
-1-#define pud_page (pud) pmd_page(pud_pmd(pud)) +#define pud_page (pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))It would be cleaner to use
pud_pfnhere, otherwise we end up passing a physical address with the lower attributes present to__phys_to_pfn, which "knows" to shift them away.OK, I tried this, together with aligning
pmd_pageto usepmd_pfn, and after debugging I realised why it is a bad idea. I had a feeling that I tried this before but didn't remember the details.I'll take the
pmd_page()example, it is used in two scenarios:a) getting the first page structure of a huge page
b) getting the page structure of the pte page pointed at by the pmdIn the first case, we know that the pmd value is aligned to
PMD_SIZEas it points to the physical address of a huge page, sopmd_pfn()masking is fine. In the second case, such masking is not, a pte page can be justPAGE_SIZEaligned. So usingpmd_pfnto mask out the bits above 12 in the pmd val breaks case (b) above.
Quoted 8 lines
While we don't have such use-case (b) for
pud_page(), I would rather keep them consistent withpmd_page.A better fix would be to change such
pmd_page()usage topmd_pgtable()in the core mm code and have a different implementation for the latter. I guess we can leave this exercise to Steve ;).In the meantime, I'll merge Jungseok's original patch.
After reading this mail, I could imagine what happen to pmd_page().
Thanks
Jungseok Lee