Message ID | 20200602125445.GA12527@alpha.franken.de (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | [GIT,PULL] MIPS changes for v5.8-rc1 | expand |
On Tue, Jun 2, 2020 at 5:55 AM Thomas Bogendoerfer <tsbogend@alpha.franken.de> wrote: > > Bibo Mao (4): > mm/memory.c: Add memory read privilege on page fault handling Hmm. That's a horribly named commit, but can you clarify why this didn't just use the existing pte_mkyoung? These are all paths that set the dirty bit early if it's a write, I don't see why they wouldn't set the accessed bit too. Even on architectures that manage the accessed bit in hardware, this might be a (tiny) performance advantage because it avoids a page table walker microfault to set the bit when it's accessed. We already do the pte_mkyoung() in paths like cow_user_page(), so the others may have been just oversights? Or was there possibly some reason we didn't want to do it here? Linus
The pull request you sent on Tue, 2 Jun 2020 14:54:45 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux tags/mips_5.8
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8226f11318bd03ae0dbf028f7c433071cf4927f4
Thank you!
On 06/04/2020 05:00 AM, Linus Torvalds wrote: > On Tue, Jun 2, 2020 at 5:55 AM Thomas Bogendoerfer > <tsbogend@alpha.franken.de> wrote: >> >> Bibo Mao (4): >> mm/memory.c: Add memory read privilege on page fault handling > > Hmm. That's a horribly named commit, but can you clarify why this > didn't just use the existing pte_mkyoung? > > These are all paths that set the dirty bit early if it's a write, I > don't see why they wouldn't set the accessed bit too. > > Even on architectures that manage the accessed bit in hardware, this > might be a (tiny) performance advantage because it avoids a page table > walker microfault to set the bit when it's accessed. On architectures that manage the access bit in hardware, access bit is set at the beginning even if there is no memory access. On MIPS system access bit is not set at the beginning, it is set in page fault handling. There are two ways to solve this, one way is to set access bit in page fault stage like mk_dirty for write accessing, this will bring out one/two cycle performance penalty for other architectures. The other way is to set access bit in the beginning like other architectures even if there is no memory access. I am ok with the former method, only that it will influence other arches just one or two cycles. For the latter I am investigating why access bit is not set at the beginning in MIPS system. regards bibo, mao > > We already do the pte_mkyoung() in paths like cow_user_page(), so the > others may have been just oversights? > > Or was there possibly some reason we didn't want to do it here? > > Linus >
On Wed, Jun 3, 2020 at 6:11 PM maobibo <maobibo@loongson.cn> wrote: > > On architectures that manage the access bit in hardware, access bit is > set at the beginning even if there is no memory access. On MIPS system > access bit is not set at the beginning, it is set in page fault handling. I had a hard time parsing that. But I think you mean that the access bit is already set in the pgprot bits on other architectures. And yeah, you seem to be right for at least x86. So yes, it seems unnecessary to set the accessed bit again, when the accessed bit is already there from the pgprot bits. Good point. > I am investigating why access bit is not set at the > beginning in MIPS system. Yeah, that does seem to be the right thing to do. On x86, we have (for example) #define PAGE_COPY __pg(__PP| 0|_USR|___A|__NX| 0| 0| 0) .. #define __P010 PAGE_COPY where that "__A" bit is the accessed bit (_PAGE_ACCESSED), just a small internal macro to make all those line up nicely. And that ends up being used for that protection_map[] array. MIPS fills in the initial protection bits in protection_map[] with dummy values, and then seems to initialize them without ever setting the accessed bit there. Not sure why that initialization then doesn't just add the pte_mkyoung.. Linus
diff --git a/arch/mips/include/asm/vermagic.h b/arch/mips/include/asm/vermagic.h index 24dc3d35161c..4d2dae0c7c57 100644 --- a/arch/mips/include/asm/vermagic.h +++ b/arch/mips/include/asm/vermagic.h @@ -8,12 +8,16 @@ #define MODULE_PROC_FAMILY "MIPS32_R1 " #elif defined CONFIG_CPU_MIPS32_R2 #define MODULE_PROC_FAMILY "MIPS32_R2 " +#elif defined CONFIG_CPU_MIPS32_R5 +#define MODULE_PROC_FAMILY "MIPS32_R5 " #elif defined CONFIG_CPU_MIPS32_R6 #define MODULE_PROC_FAMILY "MIPS32_R6 " #elif defined CONFIG_CPU_MIPS64_R1 #define MODULE_PROC_FAMILY "MIPS64_R1 " #elif defined CONFIG_CPU_MIPS64_R2 #define MODULE_PROC_FAMILY "MIPS64_R2 " +#elif defined CONFIG_CPU_MIPS64_R5 +#define MODULE_PROC_FAMILY "MIPS64_R5 " #elif defined CONFIG_CPU_MIPS64_R6 #define MODULE_PROC_FAMILY "MIPS64_R6 " #elif defined CONFIG_CPU_R3000 @@ -46,6 +50,8 @@ #define MODULE_PROC_FAMILY "LOONGSON64 " #elif defined CONFIG_CPU_CAVIUM_OCTEON #define MODULE_PROC_FAMILY "OCTEON " +#elif defined CONFIG_CPU_P5600 +#define MODULE_PROC_FAMILY "P5600 " #elif defined CONFIG_CPU_XLR #define MODULE_PROC_FAMILY "XLR " #elif defined CONFIG_CPU_XLP