Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
On 01/30/2020 07:43 PM, Christophe Leroy wrote:
>
>
> Le 30/01/2020 à 14:04, Anshuman Khandual a écrit :
>>
>> On 01/28/2020 10:35 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 28/01/2020 à 02:27, Anshuman Khandual a écrit :
diff --git a/arch/x86/include/asm/pgtable_64.h
b/arch/x86/include/asm/pgtable_64.h
index 0b6c4042942a..fb0e76d254b3 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -53,6 +53,12 @@ static inline void sync_initial_page_table(void) { }
struct mm_struct;
+#define mm_p4d_folded mm_p4d_folded
+static inline bool mm_p4d_folded(struct mm_struct *mm)
+{
+ return !pgtable_l5_enabled();
+}
+
>>>
>>> For me this should be part of another patch, it is not directly linked to
>>> the tests.
>>
>> We did discuss about this earlier and Kirril mentioned its not worth
>> a separate patch.
>>
>> https://lore.kernel.org/linux-arm-kernel/20190913091305.rkds4f3fqv3yjhjy@box/
>
> For me it would make sense to not mix this patch which implement tests, and
> changes that are needed for the test to work (or even build) on the different
> architectures.
>
> But that's up to you.
>
>>
>>>
void set_pte_vaddr_p4d(p4d_t *p4d_page, unsigned long vaddr, pte_t
new_pte);
void set_pte_vaddr_pud(pud_t *pud_page, unsigned long vaddr, pte_t
new_pte);
diff --git a/include/asm-generic/pgtable.h
b/include/asm-generic/pgtable.h
index 798ea36a0549..e0b04787e789 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -1208,6 +1208,12 @@ static inline bool arch_has_pfn_modify_check(void)
# define PAGE_KERNEL_EXEC PAGE_KERNEL
#endif
+#ifdef CONFIG_DEBUG_VM_PGTABLE
>>>
>>> Not sure it is a good idea to put that in include/asm-generic/pgtable.h
>>
>> Logically that is the right place, as it is related to page table but
>> not something platform related.
>
> I can't see any debug related features in that file.
>
>>
>>>
>>> By doing this you are forcing a rebuild of almost all files, whereas only
>>> init/main.o and mm/debug_vm_pgtable.o should be rebuilt when activating
>>> this config option.
>>
>> I agreed but whats the alternative ? We could move these into init/main.c
>> to make things simpler but will that be a right place, given its related
>> to generic page table.
>
> What about linux/mmdebug.h instead ? (I have not checked if it would reduce
> the impact, but that's where things related to CONFIG_DEBUG_VM seems to be).
>
> Otherwise, you can just create new file, for instance
> and include that file only in the init/main.c and
> mm/debug_vm_pgtable.c
IMHO it might not be wise to add yet another header file for this purpose.
Instead lets use in line with DEBUG_VM, DEBUG_VM_PGFLAGS,
DEBUG_VIRTUAL (which is also a stand alone test). A simple grep shows that
the impact of mmdebug.h would be less than generic pgtable.h header.
>
>
>
>>
>>>
+extern void debug_vm_pgtable(void);
>>>
>>> Please don't use the 'extern' keyword, it is useless and not to be used for
>>> functions declaration.
>>
>> Really ? But, there are tons of examples doing the same thing both in
>> generic and platform code as well.
>
> Yes, but how can we improve if we blindly copy the errors from the past ?
> Having tons of 'extern' doesn't mean we must add more.
>
> I think checkpatch.pl usually complains when a patch brings a new unreleval
> extern symbol.
Sure np, will drop it. But checkpatch.pl never complained.
>
>>
>>>
+#else
+static inline void debug_vm_pgtable(void) { }
+#endif
+
#endif /* !__ASSEMBLY__ */
#ifndef io_remap_pfn_range
diff --git a/init/main.c b/init/main.c
index da1bc0b60a7d..5e59e6ac0780 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1197,6 +1197,7 @@ static noinline void __init
kernel_init_freeable(void)
sched_init_smp();
page_alloc_init_late();
+ debug_vm_pgtable();
>>>
>>> Wouldn't it be better to call debug_vm_pgtable() in kernel_init() between
>>> the call to async_synchronise_full() and ftrace_free_init_mem() ?
>>
>> IIRC, proposed location is the earliest we could call debug_vm_pgtable().
>> Is there any particular benefit or reason to move it into kernel_init() ?
>
> It would avoid having it lost in the middle of drivers logs, would be close
> to the end of init, at a place we can't miss it, close to the result of other
> tests like CONFIG_DEBUG_RODATA_TEST for instance.
>
> At the moment, you have to look for it to be sure the test is done and what
> the result is.
Sure, will move it.
>
>>
>>>
/* Initialize page ext after all struct pages are initialized. */
page_ext_init();
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5ffe144c9794..7cceae923c05 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@
Re: Latest Git kernel: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device
[might be network related, so adding netdev mailing list] On 2/1/20 4:08 PM, Christian Zigotzky wrote: > Hello, > > We regularly compile and test Linux kernels every day during the merge > window. Since Thuesday we have very high CPU loads because of the avahi > daemon on our desktop Linux systems (Ubuntu, Debian etc). > > Error message: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device > > Could you please test the latest Git kernel? > > It is possible to deactivate the avahi daemon with the following lines in the > file "/etc/avahi/avahi-daemon.conf": > > use-ipv4=no > use-ipv6=no > > But this is only a temporary solution. > > Thanks, > Christian -- ~Randy
Latest Git kernel: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device
Hello, We regularly compile and test Linux kernels every day during the merge window. Since Thuesday we have very high CPU loads because of the avahi daemon on our desktop Linux systems (Ubuntu, Debian etc). Error message: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device Could you please test the latest Git kernel? It is possible to deactivate the avahi daemon with the following lines in the file "/etc/avahi/avahi-daemon.conf": use-ipv4=no use-ipv6=no But this is only a temporary solution. Thanks, Christian
[powerpc:next] BUILD SUCCESS 41196224883a64e56e0ef237c19eb837058df071
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: 41196224883a64e56e0ef237c19eb837058df071 powerpc/32s: Fix kasan_early_hash_table() for CONFIG_VMAP_STACK elapsed time: 3111m configs tested: 241 configs skipped: 25 The following configs have been built successfully. More configs may be tested in the coming days. arm allmodconfig arm allnoconfig arm allyesconfig arm at91_dt_defconfig arm efm32_defconfig arm exynos_defconfig armmulti_v5_defconfig armmulti_v7_defconfig armshmobile_defconfig arm sunxi_defconfig arm64allmodconfig arm64 allnoconfig arm64allyesconfig arm64 defconfig sparcallyesconfig parisc defconfig arc defconfig um x86_64_defconfig arc allyesconfig riscv rv32_defconfig sparc64 allyesconfig parisc b180_defconfig m68k sun3_defconfig s390defconfig microblaze mmu_defconfig um i386_defconfig ia64defconfig powerpc defconfig c6xevmc6678_defconfig microblazenommu_defconfig powerpc allnoconfig alpha defconfig mips allmodconfig i386 alldefconfig i386 allnoconfig i386 allyesconfig i386defconfig ia64 alldefconfig ia64 allmodconfig ia64 allnoconfig ia64 allyesconfig xtensa common_defconfig openriscor1ksim_defconfig nios2 3c120_defconfig xtensa iss_defconfig c6x allyesconfig nios2 10m50_defconfig openrisc simple_smp_defconfig cskydefconfig nds32 allnoconfig nds32 defconfig h8300 edosk2674_defconfig h8300h8300h-sim_defconfig h8300 h8s-sim_defconfig m68k allmodconfig m68k m5475evb_defconfig m68k multi_defconfig powerpc ppc64_defconfig powerpc rhel-kconfig mips 32r2_defconfig mips 64r6el_defconfig mips allnoconfig mips allyesconfig mips fuloong2e_defconfig mips malta_kvm_defconfig pariscallnoconfig pariscallyesonfig pariscc3000_defconfig x86_64 randconfig-a001-20200130 x86_64 randconfig-a002-20200130 x86_64 randconfig-a003-20200130 i386 randconfig-a001-20200130 i386 randconfig-a002-20200130 i386 randconfig-a003-20200130 x86_64 randconfig-a001-20200129 x86_64 randconfig-a002-20200129 x86_64 randconfig-a003-20200129 i386 randconfig-a001-20200129 i386 randconfig-a002-20200129 i386 randconfig-a003-20200129 x86_64 randconfig-a001-20200131 x86_64 randconfig-a002-20200131 x86_64 randconfig-a003-20200131 i386 randconfig-a001-20200131 i386 randconfig-a002-20200131 i386 randconfig-a003-20200131 alpharandconfig-a001-20200130 m68k randconfig-a001-20200130 mips randconfig-a001-20200130 nds32randconfig-a001-20200130 parisc randconfig-a001-20200130 riscvrandconfig-a001-20200130 alpharandconfig-a001-20200131 m68k randconfig-a001-20200131 mips randconfig-a001-20200131 nds32randconfig-a001-20200131 parisc randconfig-a001-20200131 c6x randconfig-a001-20200130 h8300randconfig-a001-20200130 microblaze randconfig-a001-20200130 nios2randconfi
Re: [PATCH] lkdtm: Test KUAP directional user access unlocks on powerpc
On Fri, Jan 31, 2020 at 05:53:14PM +1100, Russell Currey wrote: > Correct, the ACCESS_USERSPACE test does the same thing. Splitting this > into separate R and W tests makes sense, even if it is unlikely that > one would be broken without the other. That would be my preference too -- the reason it wasn't separated before was because it was one big toggle before. I just had both directions in the test out of a desire for completeness. Splitting into WRITE_USERSPACE and READ_USERSPACE seems good. Though if you want to test functionality (read while only write disabled), then I'm not sure what that should look like. Does the new user_access_begin() API provide a way to query existing state? I'll go read the series... -- Kees Cook
Re: [PATCH v2] powerpc/32s: Don't flush all TLBs when flushing one page
Le 01/02/2020 à 09:04, Christophe Leroy a écrit :
When flushing any memory range, the flushing function
flushes all TLBs.
When (start) and (end - 1) are in the same memory page,
flush that page instead.
Signed-off-by: Christophe Leroy
Reviewed-by: Segher Boessenkool
---
v2: Reworked the test as the previous one was always false (end - start was
PAGE_SIZE - 1 for a single page)
---
arch/powerpc/mm/book3s32/tlb.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 2fcd321040ff..724c0490fb17 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -79,11 +79,14 @@ static void flush_range(struct mm_struct *mm, unsigned long
start,
int count;
unsigned int ctx = mm->context.id;
+ start &= PAGE_MASK;
if (!Hash) {
- _tlbia();
+ if (end - start <= PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
return;
}
- start &= PAGE_MASK;
if (start >= end)
return;
end = (end - 1) | ~PAGE_MASK;
Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page
On Sat, Feb 01, 2020 at 03:53:12PM +0100, Christophe Leroy wrote: > >>No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it > >>sets all low bits to 1. > > > >Oh, wow, yes, I cannot read apparently. > > > >Maybe there are some ROUND_DOWN and ROUND_UP macros you could use? > > Yes but my intention was to modify the existing code as less as possible. > What do you think about version v2 of the patch ? It looked fine to me. Add my Reviewed-by: Segher Boessenkool if you want. Segher
Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page
Le 01/02/2020 à 15:06, Segher Boessenkool a écrit :
On Sat, Feb 01, 2020 at 08:27:03AM +0100, Christophe Leroy wrote:
Le 31/01/2020 à 20:38, Segher Boessenkool a écrit :
On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
Le 31/01/2020 à 16:51, Segher Boessenkool a écrit :
On Fri, Jan 31, 2020 at 03:37:34PM +, Christophe Leroy wrote:
When the range is a single page, do a page flush instead.
+ start &= PAGE_MASK;
+ end = (end - 1) | ~PAGE_MASK;
if (!Hash) {
- _tlbia();
+ if (end - start == PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
return;
}
For just one page, you get end - start == 0 actually?
Oops, good catch.
Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.
You have all low bits masked off in both start and end, so you get zero.
You could make the condion read "if (start == end)?
No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
sets all low bits to 1.
Oh, wow, yes, I cannot read apparently.
Maybe there are some ROUND_DOWN and ROUND_UP macros you could use?
Yes but my intention was to modify the existing code as less as possible.
What do you think about version v2 of the patch ?
Christophe
Re: [PATCH] powerpc/32s: Don't flush all TLBs when flushing one page
On Sat, Feb 01, 2020 at 08:27:03AM +0100, Christophe Leroy wrote:
> Le 31/01/2020 à 20:38, Segher Boessenkool a écrit :
> >On Fri, Jan 31, 2020 at 05:15:20PM +0100, Christophe Leroy wrote:
> >>Le 31/01/2020 à 16:51, Segher Boessenkool a écrit :
> >>>On Fri, Jan 31, 2020 at 03:37:34PM +, Christophe Leroy wrote:
> When the range is a single page, do a page flush instead.
> >>>
> + start &= PAGE_MASK;
> + end = (end - 1) | ~PAGE_MASK;
> if (!Hash) {
> - _tlbia();
> + if (end - start == PAGE_SIZE)
> + _tlbie(start);
> + else
> + _tlbia();
> return;
> }
> >>>
> >>>For just one page, you get end - start == 0 actually?
> >>
> >>Oops, good catch.
> >>
> >>Indeed you don't get PAGE_SIZE but (PAGE_SIZE - 1) for just one page.
> >
> >You have all low bits masked off in both start and end, so you get zero.
> >You could make the condion read "if (start == end)?
>
> No, in end the low bits are set, that's a BIT OR with ~PAGE_MASK, so it
> sets all low bits to 1.
Oh, wow, yes, I cannot read apparently.
Maybe there are some ROUND_DOWN and ROUND_UP macros you could use?
Segher
[PATCH v2] powerpc/32s: Don't flush all TLBs when flushing one page
When flushing any memory range, the flushing function
flushes all TLBs.
When (start) and (end - 1) are in the same memory page,
flush that page instead.
Signed-off-by: Christophe Leroy
---
v2: Reworked the test as the previous one was always false (end - start was
PAGE_SIZE - 1 for a single page)
---
arch/powerpc/mm/book3s32/tlb.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 2fcd321040ff..724c0490fb17 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -79,11 +79,14 @@ static void flush_range(struct mm_struct *mm, unsigned long
start,
int count;
unsigned int ctx = mm->context.id;
+ start &= PAGE_MASK;
if (!Hash) {
- _tlbia();
+ if (end - start <= PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
return;
}
- start &= PAGE_MASK;
if (start >= end)
return;
end = (end - 1) | ~PAGE_MASK;
--
2.25.0
