Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
Dave Hansen writes: > On 04/08/2014 06:32 PM, Madhavan Srinivasan wrote: >>> > In mm/Kconfig, put >>> > >>> > config FAULT_AROUND_ORDER >>> > int >>> > default 1234 if POWERPC >>> > default 4 >>> > >>> > The way you have it now, every single architecture that needs to enable >>> > this has to go put that in their Kconfig. That's madness. This way, >> I though about it and decided not to do this way because, in future, >> sub platforms of the architecture may decide to change the values. Also, >> adding an if line for each architecture with different sub platforms >> oring to it will look messy. > > I'm not sure why I'm trying here any more. You do seem quite content to > add as much cruft to ppc and every other architecture as possible. If > your theoretical scenario pops up, you simply do this in ppc: > > config ARCH_FAULT_AROUND_ORDER > int > default 999 > default 888 if OTHER_SILLY_POWERPC_SUBARCH > > But *ONLY* in the architectures that care about doing that stuff. You > leave every other architecture on the planet alone. Then, in mm/Kconfig: > > config FAULT_AROUND_ORDER > int > default ARCH_FAULT_AROUND_ORDER if ARCH_FAULT_AROUND_ORDER > default 4 > > Your way still requires going and individually touching every single > architecture's Kconfig that wants to enable fault around. That's not an > acceptable solution. Why bother with Kconfig at all? It seems like a weird indirection. And talking about future tuning seems like a separate issue, if and when someone does the work. For the moment, let's keep it simple (as below). If you really want Kconfig, then just go straight from ARCH_FAULT_AROUND_ORDER, ie: #ifdef CONFIG_ARCH_FAULT_AROUND_ORDER #define FAULT_AROUND_ORDER CONFIG_ARCH_FAULT_AROUND_ORDER #else #define FAULT_AROUND_ORDER 4 #endif Then powerpc's Kconfig defines CONFIG_ARCH_FAULT_AROUND_ORDER, and we're done. Cheers, Rusty. diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index 32e4e212b9c1..b519c5c53cfc 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -412,4 +412,7 @@ typedef struct page *pgtable_t; #include #endif /* __ASSEMBLY__ */ +/* Measured on a 4 socket Power7 system (128 Threads and 128GB memory) */ +#define FAULT_AROUND_ORDER 3 + #endif /* _ASM_POWERPC_PAGE_H */ diff --git a/mm/memory.c b/mm/memory.c index d0f0bef3be48..9aa47e9ec7ba 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3382,7 +3382,10 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address, update_mmu_cache(vma, address, pte); } +/* Archs can override, but this seems to work for x86. */ +#ifndef FAULT_AROUND_ORDER #define FAULT_AROUND_ORDER 4 +#endif #ifdef CONFIG_DEBUG_FS static unsigned int fault_around_order = FAULT_AROUND_ORDER; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
Dave Hansen dave.han...@intel.com writes: On 04/08/2014 06:32 PM, Madhavan Srinivasan wrote: In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, I though about it and decided not to do this way because, in future, sub platforms of the architecture may decide to change the values. Also, adding an if line for each architecture with different sub platforms oring to it will look messy. I'm not sure why I'm trying here any more. You do seem quite content to add as much cruft to ppc and every other architecture as possible. If your theoretical scenario pops up, you simply do this in ppc: config ARCH_FAULT_AROUND_ORDER int default 999 default 888 if OTHER_SILLY_POWERPC_SUBARCH But *ONLY* in the architectures that care about doing that stuff. You leave every other architecture on the planet alone. Then, in mm/Kconfig: config FAULT_AROUND_ORDER int default ARCH_FAULT_AROUND_ORDER if ARCH_FAULT_AROUND_ORDER default 4 Your way still requires going and individually touching every single architecture's Kconfig that wants to enable fault around. That's not an acceptable solution. Why bother with Kconfig at all? It seems like a weird indirection. And talking about future tuning seems like a separate issue, if and when someone does the work. For the moment, let's keep it simple (as below). If you really want Kconfig, then just go straight from ARCH_FAULT_AROUND_ORDER, ie: #ifdef CONFIG_ARCH_FAULT_AROUND_ORDER #define FAULT_AROUND_ORDER CONFIG_ARCH_FAULT_AROUND_ORDER #else #define FAULT_AROUND_ORDER 4 #endif Then powerpc's Kconfig defines CONFIG_ARCH_FAULT_AROUND_ORDER, and we're done. Cheers, Rusty. diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index 32e4e212b9c1..b519c5c53cfc 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -412,4 +412,7 @@ typedef struct page *pgtable_t; #include asm-generic/memory_model.h #endif /* __ASSEMBLY__ */ +/* Measured on a 4 socket Power7 system (128 Threads and 128GB memory) */ +#define FAULT_AROUND_ORDER 3 + #endif /* _ASM_POWERPC_PAGE_H */ diff --git a/mm/memory.c b/mm/memory.c index d0f0bef3be48..9aa47e9ec7ba 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3382,7 +3382,10 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address, update_mmu_cache(vma, address, pte); } +/* Archs can override, but this seems to work for x86. */ +#ifndef FAULT_AROUND_ORDER #define FAULT_AROUND_ORDER 4 +#endif #ifdef CONFIG_DEBUG_FS static unsigned int fault_around_order = FAULT_AROUND_ORDER; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Wednesday 09 April 2014 09:18 PM, Dave Hansen wrote: > On 04/09/2014 01:20 AM, Peter Zijlstra wrote: >> This still misses out on Ben's objection that its impossible to get this >> right at compile time for many kernels, since they can boot and run on >> many different subarchs. > > Completely agree. The Kconfig-time stuff should probably just be a knob > to turn it off completely, if anything. > ok. Here is my thought. So to address Ben's concern, it would be better to have this as a variable with a default value (and the platform can override ride it). And a mm/Kconfig to disable it? Kindly let me know whether this will work. Thanks for review comments. With regards Maddy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Wednesday 09 April 2014 09:18 PM, Dave Hansen wrote: On 04/09/2014 01:20 AM, Peter Zijlstra wrote: This still misses out on Ben's objection that its impossible to get this right at compile time for many kernels, since they can boot and run on many different subarchs. Completely agree. The Kconfig-time stuff should probably just be a knob to turn it off completely, if anything. ok. Here is my thought. So to address Ben's concern, it would be better to have this as a variable with a default value (and the platform can override ride it). And a mm/Kconfig to disable it? Kindly let me know whether this will work. Thanks for review comments. With regards Maddy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/09/2014 01:20 AM, Peter Zijlstra wrote: > This still misses out on Ben's objection that its impossible to get this > right at compile time for many kernels, since they can boot and run on > many different subarchs. Completely agree. The Kconfig-time stuff should probably just be a knob to turn it off completely, if anything. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/08/2014 06:32 PM, Madhavan Srinivasan wrote: >> > In mm/Kconfig, put >> > >> >config FAULT_AROUND_ORDER >> >int >> >default 1234 if POWERPC >> >default 4 >> > >> > The way you have it now, every single architecture that needs to enable >> > this has to go put that in their Kconfig. That's madness. This way, > I though about it and decided not to do this way because, in future, > sub platforms of the architecture may decide to change the values. Also, > adding an if line for each architecture with different sub platforms > oring to it will look messy. I'm not sure why I'm trying here any more. You do seem quite content to add as much cruft to ppc and every other architecture as possible. If your theoretical scenario pops up, you simply do this in ppc: config ARCH_FAULT_AROUND_ORDER int default 999 default 888 if OTHER_SILLY_POWERPC_SUBARCH But *ONLY* in the architectures that care about doing that stuff. You leave every other architecture on the planet alone. Then, in mm/Kconfig: config FAULT_AROUND_ORDER int default ARCH_FAULT_AROUND_ORDER if ARCH_FAULT_AROUND_ORDER default 4 Your way still requires going and individually touching every single architecture's Kconfig that wants to enable fault around. That's not an acceptable solution. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Wed, Apr 09, 2014 at 07:02:02AM +0530, Madhavan Srinivasan wrote: > On Friday 04 April 2014 09:48 PM, Dave Hansen wrote: > > On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: > >> This patch creates infrastructure to move the FAULT_AROUND_ORDER > >> to arch/ using Kconfig. This will enable architecture maintainers > >> to decide on suitable FAULT_AROUND_ORDER value based on > >> performance data for that architecture. Patch also adds > >> FAULT_AROUND_ORDER Kconfig element in arch/X86. > > > > Please don't do it this way. > > > > In mm/Kconfig, put > > > > config FAULT_AROUND_ORDER > > int > > default 1234 if POWERPC > > default 4 > > > > The way you have it now, every single architecture that needs to enable > > this has to go put that in their Kconfig. That's madness. This way, > > I though about it and decided not to do this way because, in future, > sub platforms of the architecture may decide to change the values. Also, > adding an if line for each architecture with different sub platforms > oring to it will look messy. This still misses out on Ben's objection that its impossible to get this right at compile time for many kernels, since they can boot and run on many different subarchs. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Wed, Apr 09, 2014 at 07:02:02AM +0530, Madhavan Srinivasan wrote: On Friday 04 April 2014 09:48 PM, Dave Hansen wrote: On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, I though about it and decided not to do this way because, in future, sub platforms of the architecture may decide to change the values. Also, adding an if line for each architecture with different sub platforms oring to it will look messy. This still misses out on Ben's objection that its impossible to get this right at compile time for many kernels, since they can boot and run on many different subarchs. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/08/2014 06:32 PM, Madhavan Srinivasan wrote: In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, I though about it and decided not to do this way because, in future, sub platforms of the architecture may decide to change the values. Also, adding an if line for each architecture with different sub platforms oring to it will look messy. I'm not sure why I'm trying here any more. You do seem quite content to add as much cruft to ppc and every other architecture as possible. If your theoretical scenario pops up, you simply do this in ppc: config ARCH_FAULT_AROUND_ORDER int default 999 default 888 if OTHER_SILLY_POWERPC_SUBARCH But *ONLY* in the architectures that care about doing that stuff. You leave every other architecture on the planet alone. Then, in mm/Kconfig: config FAULT_AROUND_ORDER int default ARCH_FAULT_AROUND_ORDER if ARCH_FAULT_AROUND_ORDER default 4 Your way still requires going and individually touching every single architecture's Kconfig that wants to enable fault around. That's not an acceptable solution. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/09/2014 01:20 AM, Peter Zijlstra wrote: This still misses out on Ben's objection that its impossible to get this right at compile time for many kernels, since they can boot and run on many different subarchs. Completely agree. The Kconfig-time stuff should probably just be a knob to turn it off completely, if anything. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 11:20 PM, David Miller wrote: > From: Dave Hansen > Date: Fri, 04 Apr 2014 09:18:43 -0700 > >> On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: >>> This patch creates infrastructure to move the FAULT_AROUND_ORDER >>> to arch/ using Kconfig. This will enable architecture maintainers >>> to decide on suitable FAULT_AROUND_ORDER value based on >>> performance data for that architecture. Patch also adds >>> FAULT_AROUND_ORDER Kconfig element in arch/X86. >> >> Please don't do it this way. >> >> In mm/Kconfig, put >> >> config FAULT_AROUND_ORDER >> int >> default 1234 if POWERPC >> default 4 >> >> The way you have it now, every single architecture that needs to enable >> this has to go put that in their Kconfig. That's madness. This way, >> you only put it in one place, and folks only have to care if they want >> to change the default to be something other than 4. > > It looks more like it's necessary only to change the default, not > to enable it. Unless I read his patch wrong... > Yes. With current patch, you only need to change the default by which you enable it. With regards Maddy > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 09:48 PM, Dave Hansen wrote: > On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: >> This patch creates infrastructure to move the FAULT_AROUND_ORDER >> to arch/ using Kconfig. This will enable architecture maintainers >> to decide on suitable FAULT_AROUND_ORDER value based on >> performance data for that architecture. Patch also adds >> FAULT_AROUND_ORDER Kconfig element in arch/X86. > > Please don't do it this way. > > In mm/Kconfig, put > > config FAULT_AROUND_ORDER > int > default 1234 if POWERPC > default 4 > > The way you have it now, every single architecture that needs to enable > this has to go put that in their Kconfig. That's madness. This way, I though about it and decided not to do this way because, in future, sub platforms of the architecture may decide to change the values. Also, adding an if line for each architecture with different sub platforms oring to it will look messy. With regards Maddy > you only put it in one place, and folks only have to care if they want > to change the default to be something other than 4. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 06:47 PM, Kirill A. Shutemov wrote: > On Fri, Apr 04, 2014 at 11:57:14AM +0530, Madhavan Srinivasan wrote: >> Kirill A. Shutemov with faultaround patchset introduced >> vm_ops->map_pages() for mapping easy accessible pages around >> fault address in hope to reduce number of minor page faults. >> >> This patch creates infrastructure to move the FAULT_AROUND_ORDER >> to arch/ using Kconfig. This will enable architecture maintainers >> to decide on suitable FAULT_AROUND_ORDER value based on >> performance data for that architecture. Patch also adds >> FAULT_AROUND_ORDER Kconfig element in arch/X86. >> >> Signed-off-by: Madhavan Srinivasan >> --- >> arch/x86/Kconfig |4 >> include/linux/mm.h |9 + >> mm/memory.c| 12 +--- >> 3 files changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index 9c0a657..5833f22 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -1177,6 +1177,10 @@ config DIRECT_GBPAGES >>support it. This can improve the kernel's performance a tiny bit by >>reducing TLB pressure. If in doubt, say "Y". >> >> +config FAULT_AROUND_ORDER >> +int >> +default "4" >> + >> # Common NUMA Features >> config NUMA >> bool "Numa Memory Allocation and Scheduler Support" >> diff --git a/include/linux/mm.h b/include/linux/mm.h >> index 0bd4359..b93c1c3 100644 >> --- a/include/linux/mm.h >> +++ b/include/linux/mm.h >> @@ -26,6 +26,15 @@ struct file_ra_state; >> struct user_struct; >> struct writeback_control; >> >> +/* >> + * Fault around order is a control knob to decide the fault around pages. >> + * Default value is set to 0UL (disabled), but the arch can override it as >> + * desired. >> + */ >> +#ifndef CONFIG_FAULT_AROUND_ORDER >> +#define CONFIG_FAULT_AROUND_ORDER 0 >> +#endif >> + > > I don't think it should be in header file: nobody except mm/memory.c cares. > Just put it instead '#define FAULT_AROUND_ORDER'. > Ok. Will do this change. >> #ifndef CONFIG_NEED_MULTIPLE_NODES /* Don't use mapnrs, do it properly */ >> extern unsigned long max_mapnr; >> >> diff --git a/mm/memory.c b/mm/memory.c >> index b02c584..22a4a89 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -3358,10 +3358,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned >> long address, >> update_mmu_cache(vma, address, pte); >> } >> >> -#define FAULT_AROUND_ORDER 4 >> - >> #ifdef CONFIG_DEBUG_FS >> -static unsigned int fault_around_order = FAULT_AROUND_ORDER; >> +static unsigned int fault_around_order = CONFIG_FAULT_AROUND_ORDER; >> >> static int fault_around_order_get(void *data, u64 *val) >> { >> @@ -3371,7 +3369,7 @@ static int fault_around_order_get(void *data, u64 *val) >> >> static int fault_around_order_set(void *data, u64 val) >> { >> -BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE); >> +BUILD_BUG_ON((1UL << CONFIG_FAULT_AROUND_ORDER) > PTRS_PER_PTE); >> if (1UL << val > PTRS_PER_PTE) >> return -EINVAL; >> fault_around_order = val; >> @@ -3406,14 +3404,14 @@ static inline unsigned long fault_around_pages(void) >> { >> unsigned long nr_pages; >> >> -nr_pages = 1UL << FAULT_AROUND_ORDER; >> +nr_pages = 1UL << CONFIG_FAULT_AROUND_ORDER; >> BUILD_BUG_ON(nr_pages > PTRS_PER_PTE); >> return nr_pages; >> } >> >> static inline unsigned long fault_around_mask(void) >> { >> -return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1); >> +return ~((1UL << (PAGE_SHIFT + CONFIG_FAULT_AROUND_ORDER)) - 1); >> } >> #endif >> >> @@ -3471,7 +3469,7 @@ static int do_read_fault(struct mm_struct *mm, struct >> vm_area_struct *vma, >> * if page by the offset is not ready to be mapped (cold cache or >> * something). >> */ >> -if (vma->vm_ops->map_pages) { >> +if ((vma->vm_ops->map_pages) && (fault_around_pages() > 1)) { > > if (vma->vm_ops->map_pages && fault_around_pages()) { > For a fault around value of 0, fault_around_pages() will return 1 and that is reason for checking it greater than 1. Also, using debug fs, fault around value can be zeroed. With regards Maddy >> pte = pte_offset_map_lock(mm, pmd, address, ); >> do_fault_around(vma, address, pte, pgoff, flags); >> if (!pte_same(*pte, orig_pte)) >> -- >> 1.7.10.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 06:47 PM, Kirill A. Shutemov wrote: On Fri, Apr 04, 2014 at 11:57:14AM +0530, Madhavan Srinivasan wrote: Kirill A. Shutemov with faultaround patchset introduced vm_ops-map_pages() for mapping easy accessible pages around fault address in hope to reduce number of minor page faults. This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Signed-off-by: Madhavan Srinivasan ma...@linux.vnet.ibm.com --- arch/x86/Kconfig |4 include/linux/mm.h |9 + mm/memory.c| 12 +--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 9c0a657..5833f22 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1177,6 +1177,10 @@ config DIRECT_GBPAGES support it. This can improve the kernel's performance a tiny bit by reducing TLB pressure. If in doubt, say Y. +config FAULT_AROUND_ORDER +int +default 4 + # Common NUMA Features config NUMA bool Numa Memory Allocation and Scheduler Support diff --git a/include/linux/mm.h b/include/linux/mm.h index 0bd4359..b93c1c3 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -26,6 +26,15 @@ struct file_ra_state; struct user_struct; struct writeback_control; +/* + * Fault around order is a control knob to decide the fault around pages. + * Default value is set to 0UL (disabled), but the arch can override it as + * desired. + */ +#ifndef CONFIG_FAULT_AROUND_ORDER +#define CONFIG_FAULT_AROUND_ORDER 0 +#endif + I don't think it should be in header file: nobody except mm/memory.c cares. Just put it instead '#define FAULT_AROUND_ORDER'. Ok. Will do this change. #ifndef CONFIG_NEED_MULTIPLE_NODES /* Don't use mapnrs, do it properly */ extern unsigned long max_mapnr; diff --git a/mm/memory.c b/mm/memory.c index b02c584..22a4a89 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3358,10 +3358,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address, update_mmu_cache(vma, address, pte); } -#define FAULT_AROUND_ORDER 4 - #ifdef CONFIG_DEBUG_FS -static unsigned int fault_around_order = FAULT_AROUND_ORDER; +static unsigned int fault_around_order = CONFIG_FAULT_AROUND_ORDER; static int fault_around_order_get(void *data, u64 *val) { @@ -3371,7 +3369,7 @@ static int fault_around_order_get(void *data, u64 *val) static int fault_around_order_set(void *data, u64 val) { -BUILD_BUG_ON((1UL FAULT_AROUND_ORDER) PTRS_PER_PTE); +BUILD_BUG_ON((1UL CONFIG_FAULT_AROUND_ORDER) PTRS_PER_PTE); if (1UL val PTRS_PER_PTE) return -EINVAL; fault_around_order = val; @@ -3406,14 +3404,14 @@ static inline unsigned long fault_around_pages(void) { unsigned long nr_pages; -nr_pages = 1UL FAULT_AROUND_ORDER; +nr_pages = 1UL CONFIG_FAULT_AROUND_ORDER; BUILD_BUG_ON(nr_pages PTRS_PER_PTE); return nr_pages; } static inline unsigned long fault_around_mask(void) { -return ~((1UL (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1); +return ~((1UL (PAGE_SHIFT + CONFIG_FAULT_AROUND_ORDER)) - 1); } #endif @@ -3471,7 +3469,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma, * if page by the offset is not ready to be mapped (cold cache or * something). */ -if (vma-vm_ops-map_pages) { +if ((vma-vm_ops-map_pages) (fault_around_pages() 1)) { if (vma-vm_ops-map_pages fault_around_pages()) { For a fault around value of 0, fault_around_pages() will return 1 and that is reason for checking it greater than 1. Also, using debug fs, fault around value can be zeroed. With regards Maddy pte = pte_offset_map_lock(mm, pmd, address, ptl); do_fault_around(vma, address, pte, pgoff, flags); if (!pte_same(*pte, orig_pte)) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 09:48 PM, Dave Hansen wrote: On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, I though about it and decided not to do this way because, in future, sub platforms of the architecture may decide to change the values. Also, adding an if line for each architecture with different sub platforms oring to it will look messy. With regards Maddy you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Friday 04 April 2014 11:20 PM, David Miller wrote: From: Dave Hansen dave.han...@intel.com Date: Fri, 04 Apr 2014 09:18:43 -0700 On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. It looks more like it's necessary only to change the default, not to enable it. Unless I read his patch wrong... Yes. With current patch, you only need to change the default by which you enable it. With regards Maddy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Fri, 2014-04-04 at 09:18 -0700, Dave Hansen wrote: > On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: > > This patch creates infrastructure to move the FAULT_AROUND_ORDER > > to arch/ using Kconfig. This will enable architecture maintainers > > to decide on suitable FAULT_AROUND_ORDER value based on > > performance data for that architecture. Patch also adds > > FAULT_AROUND_ORDER Kconfig element in arch/X86. > > Please don't do it this way. > > In mm/Kconfig, put > > config FAULT_AROUND_ORDER > int > default 1234 if POWERPC > default 4 > > The way you have it now, every single architecture that needs to enable > this has to go put that in their Kconfig. That's madness. This way, > you only put it in one place, and folks only have to care if they want > to change the default to be something other than 4. Also does it have to be a constant ? Maddy here tested on our POWER servers. The "Sweet spot" value might be VERY different on an embedded chip or even on a future generation of server chip. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Fri, 2014-04-04 at 09:18 -0700, Dave Hansen wrote: On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. Also does it have to be a constant ? Maddy here tested on our POWER servers. The Sweet spot value might be VERY different on an embedded chip or even on a future generation of server chip. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Dave Hansen Date: Fri, 04 Apr 2014 09:18:43 -0700 > On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: >> This patch creates infrastructure to move the FAULT_AROUND_ORDER >> to arch/ using Kconfig. This will enable architecture maintainers >> to decide on suitable FAULT_AROUND_ORDER value based on >> performance data for that architecture. Patch also adds >> FAULT_AROUND_ORDER Kconfig element in arch/X86. > > Please don't do it this way. > > In mm/Kconfig, put > > config FAULT_AROUND_ORDER > int > default 1234 if POWERPC > default 4 > > The way you have it now, every single architecture that needs to enable > this has to go put that in their Kconfig. That's madness. This way, > you only put it in one place, and folks only have to care if they want > to change the default to be something other than 4. It looks more like it's necessary only to change the default, not to enable it. Unless I read his patch wrong... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: > This patch creates infrastructure to move the FAULT_AROUND_ORDER > to arch/ using Kconfig. This will enable architecture maintainers > to decide on suitable FAULT_AROUND_ORDER value based on > performance data for that architecture. Patch also adds > FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Fri, Apr 04, 2014 at 11:57:14AM +0530, Madhavan Srinivasan wrote: > Kirill A. Shutemov with faultaround patchset introduced > vm_ops->map_pages() for mapping easy accessible pages around > fault address in hope to reduce number of minor page faults. > > This patch creates infrastructure to move the FAULT_AROUND_ORDER > to arch/ using Kconfig. This will enable architecture maintainers > to decide on suitable FAULT_AROUND_ORDER value based on > performance data for that architecture. Patch also adds > FAULT_AROUND_ORDER Kconfig element in arch/X86. > > Signed-off-by: Madhavan Srinivasan > --- > arch/x86/Kconfig |4 > include/linux/mm.h |9 + > mm/memory.c| 12 +--- > 3 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 9c0a657..5833f22 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1177,6 +1177,10 @@ config DIRECT_GBPAGES > support it. This can improve the kernel's performance a tiny bit by > reducing TLB pressure. If in doubt, say "Y". > > +config FAULT_AROUND_ORDER > + int > + default "4" > + > # Common NUMA Features > config NUMA > bool "Numa Memory Allocation and Scheduler Support" > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 0bd4359..b93c1c3 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -26,6 +26,15 @@ struct file_ra_state; > struct user_struct; > struct writeback_control; > > +/* > + * Fault around order is a control knob to decide the fault around pages. > + * Default value is set to 0UL (disabled), but the arch can override it as > + * desired. > + */ > +#ifndef CONFIG_FAULT_AROUND_ORDER > +#define CONFIG_FAULT_AROUND_ORDER 0 > +#endif > + I don't think it should be in header file: nobody except mm/memory.c cares. Just put it instead '#define FAULT_AROUND_ORDER'. > #ifndef CONFIG_NEED_MULTIPLE_NODES /* Don't use mapnrs, do it properly */ > extern unsigned long max_mapnr; > > diff --git a/mm/memory.c b/mm/memory.c > index b02c584..22a4a89 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3358,10 +3358,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned > long address, > update_mmu_cache(vma, address, pte); > } > > -#define FAULT_AROUND_ORDER 4 > - > #ifdef CONFIG_DEBUG_FS > -static unsigned int fault_around_order = FAULT_AROUND_ORDER; > +static unsigned int fault_around_order = CONFIG_FAULT_AROUND_ORDER; > > static int fault_around_order_get(void *data, u64 *val) > { > @@ -3371,7 +3369,7 @@ static int fault_around_order_get(void *data, u64 *val) > > static int fault_around_order_set(void *data, u64 val) > { > - BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE); > + BUILD_BUG_ON((1UL << CONFIG_FAULT_AROUND_ORDER) > PTRS_PER_PTE); > if (1UL << val > PTRS_PER_PTE) > return -EINVAL; > fault_around_order = val; > @@ -3406,14 +3404,14 @@ static inline unsigned long fault_around_pages(void) > { > unsigned long nr_pages; > > - nr_pages = 1UL << FAULT_AROUND_ORDER; > + nr_pages = 1UL << CONFIG_FAULT_AROUND_ORDER; > BUILD_BUG_ON(nr_pages > PTRS_PER_PTE); > return nr_pages; > } > > static inline unsigned long fault_around_mask(void) > { > - return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1); > + return ~((1UL << (PAGE_SHIFT + CONFIG_FAULT_AROUND_ORDER)) - 1); > } > #endif > > @@ -3471,7 +3469,7 @@ static int do_read_fault(struct mm_struct *mm, struct > vm_area_struct *vma, >* if page by the offset is not ready to be mapped (cold cache or >* something). >*/ > - if (vma->vm_ops->map_pages) { > + if ((vma->vm_ops->map_pages) && (fault_around_pages() > 1)) { if (vma->vm_ops->map_pages && fault_around_pages()) { > pte = pte_offset_map_lock(mm, pmd, address, ); > do_fault_around(vma, address, pte, pgoff, flags); > if (!pte_same(*pte, orig_pte)) > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Kirill A. Shutemov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On Fri, Apr 04, 2014 at 11:57:14AM +0530, Madhavan Srinivasan wrote: Kirill A. Shutemov with faultaround patchset introduced vm_ops-map_pages() for mapping easy accessible pages around fault address in hope to reduce number of minor page faults. This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Signed-off-by: Madhavan Srinivasan ma...@linux.vnet.ibm.com --- arch/x86/Kconfig |4 include/linux/mm.h |9 + mm/memory.c| 12 +--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 9c0a657..5833f22 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1177,6 +1177,10 @@ config DIRECT_GBPAGES support it. This can improve the kernel's performance a tiny bit by reducing TLB pressure. If in doubt, say Y. +config FAULT_AROUND_ORDER + int + default 4 + # Common NUMA Features config NUMA bool Numa Memory Allocation and Scheduler Support diff --git a/include/linux/mm.h b/include/linux/mm.h index 0bd4359..b93c1c3 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -26,6 +26,15 @@ struct file_ra_state; struct user_struct; struct writeback_control; +/* + * Fault around order is a control knob to decide the fault around pages. + * Default value is set to 0UL (disabled), but the arch can override it as + * desired. + */ +#ifndef CONFIG_FAULT_AROUND_ORDER +#define CONFIG_FAULT_AROUND_ORDER 0 +#endif + I don't think it should be in header file: nobody except mm/memory.c cares. Just put it instead '#define FAULT_AROUND_ORDER'. #ifndef CONFIG_NEED_MULTIPLE_NODES /* Don't use mapnrs, do it properly */ extern unsigned long max_mapnr; diff --git a/mm/memory.c b/mm/memory.c index b02c584..22a4a89 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3358,10 +3358,8 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address, update_mmu_cache(vma, address, pte); } -#define FAULT_AROUND_ORDER 4 - #ifdef CONFIG_DEBUG_FS -static unsigned int fault_around_order = FAULT_AROUND_ORDER; +static unsigned int fault_around_order = CONFIG_FAULT_AROUND_ORDER; static int fault_around_order_get(void *data, u64 *val) { @@ -3371,7 +3369,7 @@ static int fault_around_order_get(void *data, u64 *val) static int fault_around_order_set(void *data, u64 val) { - BUILD_BUG_ON((1UL FAULT_AROUND_ORDER) PTRS_PER_PTE); + BUILD_BUG_ON((1UL CONFIG_FAULT_AROUND_ORDER) PTRS_PER_PTE); if (1UL val PTRS_PER_PTE) return -EINVAL; fault_around_order = val; @@ -3406,14 +3404,14 @@ static inline unsigned long fault_around_pages(void) { unsigned long nr_pages; - nr_pages = 1UL FAULT_AROUND_ORDER; + nr_pages = 1UL CONFIG_FAULT_AROUND_ORDER; BUILD_BUG_ON(nr_pages PTRS_PER_PTE); return nr_pages; } static inline unsigned long fault_around_mask(void) { - return ~((1UL (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1); + return ~((1UL (PAGE_SHIFT + CONFIG_FAULT_AROUND_ORDER)) - 1); } #endif @@ -3471,7 +3469,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma, * if page by the offset is not ready to be mapped (cold cache or * something). */ - if (vma-vm_ops-map_pages) { + if ((vma-vm_ops-map_pages) (fault_around_pages() 1)) { if (vma-vm_ops-map_pages fault_around_pages()) { pte = pte_offset_map_lock(mm, pmd, address, ptl); do_fault_around(vma, address, pte, pgoff, flags); if (!pte_same(*pte, orig_pte)) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Kirill A. Shutemov -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Dave Hansen dave.han...@intel.com Date: Fri, 04 Apr 2014 09:18:43 -0700 On 04/03/2014 11:27 PM, Madhavan Srinivasan wrote: This patch creates infrastructure to move the FAULT_AROUND_ORDER to arch/ using Kconfig. This will enable architecture maintainers to decide on suitable FAULT_AROUND_ORDER value based on performance data for that architecture. Patch also adds FAULT_AROUND_ORDER Kconfig element in arch/X86. Please don't do it this way. In mm/Kconfig, put config FAULT_AROUND_ORDER int default 1234 if POWERPC default 4 The way you have it now, every single architecture that needs to enable this has to go put that in their Kconfig. That's madness. This way, you only put it in one place, and folks only have to care if they want to change the default to be something other than 4. It looks more like it's necessary only to change the default, not to enable it. Unless I read his patch wrong... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/