| Message ID | 1657181495-33004-1-git-send-email-zhanghongchen@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: fix pmd_mkinvalid | expand |
On Thu, Jul 07, 2022 at 04:11:35PM +0800, Hongchen Zhang wrote: > When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should > return true. > So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is > present but invalidated by pmd_mkinvalid. What problem are you trying to fix ? What are the symptoms ? > Reported-by: kernel test robot <lkp@intel.com> the test robot showed problems with your last version of the patch, which hasn't been integrated into at least the MIPS tree, so no need to that. Thomas.
On 2022/7/7 下午5:22, Thomas Bogendoerfer wrote: > On Thu, Jul 07, 2022 at 04:11:35PM +0800, Hongchen Zhang wrote: >> When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should >> return true. >> So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is >> present but invalidated by pmd_mkinvalid. > > What problem are you trying to fix ? What are the symptoms ? > >> Reported-by: kernel test robot <lkp@intel.com> > > the test robot showed problems with your last version of the patch, > which hasn't been integrated into at least the MIPS tree, so no > need to that. > > Thomas. > Hi Thomas, The idea come from the commit: b65399f6111b(arm64/mm: Change THP helpers to comply with generic MM semantics). There is an problem now: CPU 0 CPU 1 pmdp_invalidate do_page_fault ... __handle_mm_fault is_swap_pmd == true trigger VM_BUG_ON() ? set_pmd_at the reason is that pmd_present return true,after this commit pmd_present will return false,and the VM_BUG_ON will not be triggered. Like arm64 does,we can introduce a new bit to fix this. Thanks.
On 2022/7/7 下午7:12, Hongchen Zhang wrote: > On 2022/7/7 下午5:22, Thomas Bogendoerfer wrote: >> On Thu, Jul 07, 2022 at 04:11:35PM +0800, Hongchen Zhang wrote: >>> When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should >>> return true. >>> So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is >>> present but invalidated by pmd_mkinvalid. >> >> What problem are you trying to fix ? What are the symptoms ? >> >>> Reported-by: kernel test robot <lkp@intel.com> >> >> the test robot showed problems with your last version of the patch, >> which hasn't been integrated into at least the MIPS tree, so no >> need to that. >> >> Thomas. >> > > Hi Thomas, > The idea come from the commit: > b65399f6111b(arm64/mm: Change THP helpers to comply with generic MM > semantics). > There is an problem now: > CPU 0 CPU 1 > pmdp_invalidate do_page_fault > ... __handle_mm_fault > is_swap_pmd == true > trigger VM_BUG_ON() ? > set_pmd_at > the reason is that pmd_present return true,after this commit > pmd_present will return false,and the VM_BUG_ON will not be triggered. > Like arm64 does,we can introduce a new bit to fix this. > > Thanks. Hi Thomas, Is there problem of this patch? What's your opinion of this patch? Thanks
On Tue, Jul 12, 2022 at 06:01:08PM +0800, Hongchen Zhang wrote: > On 2022/7/7 下午7:12, Hongchen Zhang wrote: > > On 2022/7/7 下午5:22, Thomas Bogendoerfer wrote: > > > On Thu, Jul 07, 2022 at 04:11:35PM +0800, Hongchen Zhang wrote: > > > > When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should > > > > return true. > > > > So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is > > > > present but invalidated by pmd_mkinvalid. > > > > > > What problem are you trying to fix ? What are the symptoms ? > > > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > > the test robot showed problems with your last version of the patch, > > > which hasn't been integrated into at least the MIPS tree, so no > > > need to that. > > > > > > Thomas. > > > > > > > Hi Thomas, > > The idea come from the commit: > > b65399f6111b(arm64/mm: Change THP helpers to comply with generic MM > > semantics). > > There is an problem now: > > CPU 0 CPU 1 > > pmdp_invalidate do_page_fault > > ... __handle_mm_fault > > is_swap_pmd == true > > trigger VM_BUG_ON() ? > > set_pmd_at > > the reason is that pmd_present return true,after this commit > > pmd_present will return false,and the VM_BUG_ON will not be triggered. > > Like arm64 does,we can introduce a new bit to fix this. > > > > Thanks. > Hi Thomas, > Is there problem of this patch? What's your opinion of this patch? I haven't dig deeper into it, but needing more page bits is a pain for 32bit kernel and would make it nearly impossible to get huge page support there. And the description you gave me, needs to be in the commit description. Thomas.
On 2022/7/12 下午7:19, Thomas Bogendoerfer wrote: > On Tue, Jul 12, 2022 at 06:01:08PM +0800, Hongchen Zhang wrote: >> On 2022/7/7 下午7:12, Hongchen Zhang wrote: >>> On 2022/7/7 下午5:22, Thomas Bogendoerfer wrote: >>>> On Thu, Jul 07, 2022 at 04:11:35PM +0800, Hongchen Zhang wrote: >>>>> When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should >>>>> return true. >>>>> So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is >>>>> present but invalidated by pmd_mkinvalid. >>>> >>>> What problem are you trying to fix ? What are the symptoms ? >>>> >>>>> Reported-by: kernel test robot <lkp@intel.com> >>>> >>>> the test robot showed problems with your last version of the patch, >>>> which hasn't been integrated into at least the MIPS tree, so no >>>> need to that. >>>> >>>> Thomas. >>>> >>> >>> Hi Thomas, >>> The idea come from the commit: >>> b65399f6111b(arm64/mm: Change THP helpers to comply with generic MM >>> semantics). >>> There is an problem now: >>> CPU 0 CPU 1 >>> pmdp_invalidate do_page_fault >>> ... __handle_mm_fault >>> is_swap_pmd == true >>> trigger VM_BUG_ON() ? >>> set_pmd_at >>> the reason is that pmd_present return true,after this commit >>> pmd_present will return false,and the VM_BUG_ON will not be triggered. >>> Like arm64 does,we can introduce a new bit to fix this. >>> >>> Thanks. >> Hi Thomas, >> Is there problem of this patch? What's your opinion of this patch? > > I haven't dig deeper into it, but needing more page bits is a pain > for 32bit kernel and would make it nearly impossible to get huge > page support there. And the description you gave me, needs to be > in the commit description. > > Thomas. > Hi Thomas, Thanks for your patiently review. For your question, 1. I think there may be problem when compile 32bit kernel with huge page support,because _PAGE_HUGE_SHIFT is only defined for R4K now. 2. I will modify the commit as you said and make a v2 patch. Thanks. Hongchen Zhang
diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h index 41921ac..050cf66 100644 --- a/arch/mips/include/asm/pgtable-64.h +++ b/arch/mips/include/asm/pgtable-64.h @@ -265,7 +265,7 @@ static inline int pmd_present(pmd_t pmd) { #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT if (unlikely(pmd_val(pmd) & _PAGE_HUGE)) - return pmd_val(pmd) & _PAGE_PRESENT; + return pmd_val(pmd) & (_PAGE_PRESENT | _PMD_PRESENT_INVALID); #endif return pmd_val(pmd) != (unsigned long) invalid_pte_table; diff --git a/arch/mips/include/asm/pgtable-bits.h b/arch/mips/include/asm/pgtable-bits.h index 2362842..72cd88a 100644 --- a/arch/mips/include/asm/pgtable-bits.h +++ b/arch/mips/include/asm/pgtable-bits.h @@ -130,6 +130,7 @@ enum pgtable_bits { _PAGE_MODIFIED_SHIFT, #if defined(CONFIG_MIPS_HUGE_TLB_SUPPORT) _PAGE_HUGE_SHIFT, + _PMD_PRESENT_INVALID_SHIFT, #endif #if defined(CONFIG_ARCH_HAS_PTE_SPECIAL) _PAGE_SPECIAL_SHIFT, @@ -157,6 +158,7 @@ enum pgtable_bits { #define _PAGE_MODIFIED (1 << _PAGE_MODIFIED_SHIFT) #if defined(CONFIG_MIPS_HUGE_TLB_SUPPORT) # define _PAGE_HUGE (1 << _PAGE_HUGE_SHIFT) +#define _PMD_PRESENT_INVALID (1 << _PMD_PRESENT_INVALID_SHIFT) #endif #if defined(CONFIG_ARCH_HAS_PTE_SPECIAL) # define _PAGE_SPECIAL (1 << _PAGE_SPECIAL_SHIFT) diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h index 374c632..a75f461 100644 --- a/arch/mips/include/asm/pgtable.h +++ b/arch/mips/include/asm/pgtable.h @@ -696,12 +696,15 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot) return pmd; } +#ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT static inline pmd_t pmd_mkinvalid(pmd_t pmd) { + pmd_val(pmd) |= _PMD_PRESENT_INVALID; pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY); return pmd; } +#endif /* * The generic version pmdp_huge_get_and_clear uses a version of pmd_clear() with a
When a pmd entry is invalidated by pmd_mkinvalid,pmd_present should return true. So introduce a _PMD_PRESENT_INVALID_SHIFT bit to check if a pmd is present but invalidated by pmd_mkinvalid. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Hongchen Zhang <zhanghongchen@loongson.cn> --- arch/mips/include/asm/pgtable-64.h | 2 +- arch/mips/include/asm/pgtable-bits.h | 2 ++ arch/mips/include/asm/pgtable.h | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-)