Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
Le 18/09/2019 à 09:32, Anshuman Khandual a écrit : On 09/13/2019 11:53 AM, Christophe Leroy wrote: Fix build failure on powerpc. Fix preemption imbalance. Signed-off-by: Christophe Leroy --- mm/arch_pgtable_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c index 8b4a92756ad8..f2b3c9ec35fa 100644 --- a/mm/arch_pgtable_test.c +++ b/mm/arch_pgtable_test.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) p4d_clear_tests(p4dp); pgd_clear_tests(mm, pgdp); + pte_unmap(ptep); + pmd_populate_tests(mm, pmdp, saved_ptep); pud_populate_tests(mm, pudp, saved_pmdp); p4d_populate_tests(mm, p4dp, saved_pudp); Hello Christophe, I am planning to fold this fix into the current patch and retain your Signed-off-by. Are you okay with it ? No problem, do whatever is convenient for you. You can keep the signed-off-by, or use tested-by: as I tested it on PPC32. Christophe
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
On 09/13/2019 11:53 AM, Christophe Leroy wrote: > Fix build failure on powerpc. > > Fix preemption imbalance. > > Signed-off-by: Christophe Leroy > --- > mm/arch_pgtable_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c > index 8b4a92756ad8..f2b3c9ec35fa 100644 > --- a/mm/arch_pgtable_test.c > +++ b/mm/arch_pgtable_test.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) > p4d_clear_tests(p4dp); > pgd_clear_tests(mm, pgdp); > > + pte_unmap(ptep); > + > pmd_populate_tests(mm, pmdp, saved_ptep); > pud_populate_tests(mm, pudp, saved_pmdp); > p4d_populate_tests(mm, p4dp, saved_pudp); > Hello Christophe, I am planning to fold this fix into the current patch and retain your Signed-off-by. Are you okay with it ? - Anshuman
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
On Fri, Sep 13, 2019 at 02:12:45PM +0530, Anshuman Khandual wrote: > > > On 09/13/2019 12:41 PM, Christophe Leroy wrote: > > > > > > Le 13/09/2019 à 09:03, Christophe Leroy a écrit : > >> > >> > >> Le 13/09/2019 à 08:58, Anshuman Khandual a écrit : > >>> On 09/13/2019 11:53 AM, Christophe Leroy wrote: > Fix build failure on powerpc. > > Fix preemption imbalance. > > Signed-off-by: Christophe Leroy > --- > mm/arch_pgtable_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c > index 8b4a92756ad8..f2b3c9ec35fa 100644 > --- a/mm/arch_pgtable_test.c > +++ b/mm/arch_pgtable_test.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > >>> > >>> This is okay. > >>> > #include > #include > @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) > p4d_clear_tests(p4dp); > pgd_clear_tests(mm, pgdp); > + pte_unmap(ptep); > + > >>> > >>> Now the preemption imbalance via pte_alloc_map() path i.e > >>> > >>> pte_alloc_map() -> pte_offset_map() -> kmap_atomic() > >>> > >>> Is not this very much powerpc 32 specific or this will be applicable > >>> for all platform which uses kmap_XXX() to map high memory ? > >>> > >> > >> See > >> https://elixir.bootlin.com/linux/v5.3-rc8/source/include/linux/highmem.h#L91 > >> > >> I think it applies at least to all arches using the generic implementation. > >> > >> Applies also to arm: > >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/mm/highmem.c#L52 > >> > >> Applies also to mips: > >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/mips/mm/highmem.c#L47 > >> > >> Same on sparc: > >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/sparc/mm/highmem.c#L52 > >> > >> Same on x86: > >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/mm/highmem_32.c#L34 > >> > >> I have not checked others, but I guess it is like that for all. > >> > > > > > > Seems like I answered too quickly. All kmap_atomic() do preempt_disable(), > > but not all pte_alloc_map() call kmap_atomic(). > > > > However, for instance ARM does: > > > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/include/asm/pgtable.h#L200 > > > > And X86 as well: > > > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/include/asm/pgtable_32.h#L51 > > > > Microblaze also: > > > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/microblaze/include/asm/pgtable.h#L495 > > All the above platforms checks out to be using k[un]map_atomic(). I am > wondering whether > any of the intermediate levels will have similar problems on any these 32 bit > platforms > or any other platforms which might be using generic k[un]map_atomic(). No. Kernel only allocates pte page table from highmem. All other page tables are always visible in kernel address space. -- Kirill A. Shutemov
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
On 09/13/2019 12:41 PM, Christophe Leroy wrote: > > > Le 13/09/2019 à 09:03, Christophe Leroy a écrit : >> >> >> Le 13/09/2019 à 08:58, Anshuman Khandual a écrit : >>> On 09/13/2019 11:53 AM, Christophe Leroy wrote: Fix build failure on powerpc. Fix preemption imbalance. Signed-off-by: Christophe Leroy --- mm/arch_pgtable_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c index 8b4a92756ad8..f2b3c9ec35fa 100644 --- a/mm/arch_pgtable_test.c +++ b/mm/arch_pgtable_test.c @@ -24,6 +24,7 @@ #include #include #include +#include >>> >>> This is okay. >>> #include #include @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) p4d_clear_tests(p4dp); pgd_clear_tests(mm, pgdp); + pte_unmap(ptep); + >>> >>> Now the preemption imbalance via pte_alloc_map() path i.e >>> >>> pte_alloc_map() -> pte_offset_map() -> kmap_atomic() >>> >>> Is not this very much powerpc 32 specific or this will be applicable >>> for all platform which uses kmap_XXX() to map high memory ? >>> >> >> See >> https://elixir.bootlin.com/linux/v5.3-rc8/source/include/linux/highmem.h#L91 >> >> I think it applies at least to all arches using the generic implementation. >> >> Applies also to arm: >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/mm/highmem.c#L52 >> >> Applies also to mips: >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/mips/mm/highmem.c#L47 >> >> Same on sparc: >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/sparc/mm/highmem.c#L52 >> >> Same on x86: >> https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/mm/highmem_32.c#L34 >> >> I have not checked others, but I guess it is like that for all. >> > > > Seems like I answered too quickly. All kmap_atomic() do preempt_disable(), > but not all pte_alloc_map() call kmap_atomic(). > > However, for instance ARM does: > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/include/asm/pgtable.h#L200 > > And X86 as well: > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/include/asm/pgtable_32.h#L51 > > Microblaze also: > > https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/microblaze/include/asm/pgtable.h#L495 All the above platforms checks out to be using k[un]map_atomic(). I am wondering whether any of the intermediate levels will have similar problems on any these 32 bit platforms or any other platforms which might be using generic k[un]map_atomic(). There can be many permutations here. p4dp = p4d_alloc(mm, pgdp, vaddr); pudp = pud_alloc(mm, p4dp, vaddr); pmdp = pmd_alloc(mm, pudp, vaddr); Otherwise pte_alloc_map()/pte_unmap() looks good enough which will atleast take care of a known failure.
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
Le 13/09/2019 à 09:03, Christophe Leroy a écrit : Le 13/09/2019 à 08:58, Anshuman Khandual a écrit : On 09/13/2019 11:53 AM, Christophe Leroy wrote: Fix build failure on powerpc. Fix preemption imbalance. Signed-off-by: Christophe Leroy --- mm/arch_pgtable_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c index 8b4a92756ad8..f2b3c9ec35fa 100644 --- a/mm/arch_pgtable_test.c +++ b/mm/arch_pgtable_test.c @@ -24,6 +24,7 @@ #include #include #include +#include This is okay. #include #include @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) p4d_clear_tests(p4dp); pgd_clear_tests(mm, pgdp); + pte_unmap(ptep); + Now the preemption imbalance via pte_alloc_map() path i.e pte_alloc_map() -> pte_offset_map() -> kmap_atomic() Is not this very much powerpc 32 specific or this will be applicable for all platform which uses kmap_XXX() to map high memory ? See https://elixir.bootlin.com/linux/v5.3-rc8/source/include/linux/highmem.h#L91 I think it applies at least to all arches using the generic implementation. Applies also to arm: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/mm/highmem.c#L52 Applies also to mips: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/mips/mm/highmem.c#L47 Same on sparc: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/sparc/mm/highmem.c#L52 Same on x86: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/mm/highmem_32.c#L34 I have not checked others, but I guess it is like that for all. Seems like I answered too quickly. All kmap_atomic() do preempt_disable(), but not all pte_alloc_map() call kmap_atomic(). However, for instance ARM does: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/include/asm/pgtable.h#L200 And X86 as well: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/include/asm/pgtable_32.h#L51 Microblaze also: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/microblaze/include/asm/pgtable.h#L495 Christophe
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
Le 13/09/2019 à 08:58, Anshuman Khandual a écrit : On 09/13/2019 11:53 AM, Christophe Leroy wrote: Fix build failure on powerpc. Fix preemption imbalance. Signed-off-by: Christophe Leroy --- mm/arch_pgtable_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c index 8b4a92756ad8..f2b3c9ec35fa 100644 --- a/mm/arch_pgtable_test.c +++ b/mm/arch_pgtable_test.c @@ -24,6 +24,7 @@ #include #include #include +#include This is okay. #include #include @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) p4d_clear_tests(p4dp); pgd_clear_tests(mm, pgdp); + pte_unmap(ptep); + Now the preemption imbalance via pte_alloc_map() path i.e pte_alloc_map() -> pte_offset_map() -> kmap_atomic() Is not this very much powerpc 32 specific or this will be applicable for all platform which uses kmap_XXX() to map high memory ? See https://elixir.bootlin.com/linux/v5.3-rc8/source/include/linux/highmem.h#L91 I think it applies at least to all arches using the generic implementation. Applies also to arm: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/arm/mm/highmem.c#L52 Applies also to mips: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/mips/mm/highmem.c#L47 Same on sparc: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/sparc/mm/highmem.c#L52 Same on x86: https://elixir.bootlin.com/linux/v5.3-rc8/source/arch/x86/mm/highmem_32.c#L34 I have not checked others, but I guess it is like that for all. Christophe
Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
On 09/13/2019 11:53 AM, Christophe Leroy wrote: > Fix build failure on powerpc. > > Fix preemption imbalance. > > Signed-off-by: Christophe Leroy > --- > mm/arch_pgtable_test.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c > index 8b4a92756ad8..f2b3c9ec35fa 100644 > --- a/mm/arch_pgtable_test.c > +++ b/mm/arch_pgtable_test.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include This is okay. > #include > #include > > @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void) > p4d_clear_tests(p4dp); > pgd_clear_tests(mm, pgdp); > > + pte_unmap(ptep); > + Now the preemption imbalance via pte_alloc_map() path i.e pte_alloc_map() -> pte_offset_map() -> kmap_atomic() Is not this very much powerpc 32 specific or this will be applicable for all platform which uses kmap_XXX() to map high memory ?
