| Message ID | 1523975611-15978-3-git-send-email-ldufour@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Speculative page faults | expand |
Hi Laurent, Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: > Set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT which turns on the > Speculative Page Fault handler when building for 64bit. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> > --- > arch/x86/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index d8983df5a2bc..ebdeb48e4a4a 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -30,6 +30,7 @@ config X86_64 > select MODULES_USE_ELF_RELA > select X86_DEV_DMA_OPS > select ARCH_HAS_SYSCALL_WRAPPER > + select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT I'd suggest merging this patch with the one making changes to the architectural fault handler towards the end of the series. The Kconfig change is closely tied to the architectural support for SPF and makes sense to be in a single patch. If there's a good reason to keep them as separate patches, please move the architecture Kconfig changes after the patch adding fault handler changes. It's better to enable the feature once the core infrastructure is merged rather than at the beginning of the series to avoid potential bad fallout from incomplete functionality during bisection. All the comments here definitely hold for the arm64 patches that you plan to include with the next update. Thanks, Punit > > # > # Arch settings
On 08/05/2018 13:04, Punit Agrawal wrote: > Hi Laurent, > > Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: > >> Set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT which turns on the >> Speculative Page Fault handler when building for 64bit. >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> >> --- >> arch/x86/Kconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >> index d8983df5a2bc..ebdeb48e4a4a 100644 >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -30,6 +30,7 @@ config X86_64 >> select MODULES_USE_ELF_RELA >> select X86_DEV_DMA_OPS >> select ARCH_HAS_SYSCALL_WRAPPER >> + select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT > > I'd suggest merging this patch with the one making changes to the > architectural fault handler towards the end of the series. > > The Kconfig change is closely tied to the architectural support for SPF > and makes sense to be in a single patch. > > If there's a good reason to keep them as separate patches, please move > the architecture Kconfig changes after the patch adding fault handler > changes. > > It's better to enable the feature once the core infrastructure is merged > rather than at the beginning of the series to avoid potential bad > fallout from incomplete functionality during bisection. Indeed bisection was the reason why Andrew asked me to push the configuration enablement on top of the series (https://lkml.org/lkml/2017/10/10/1229). I also think it would be better to have the architecture enablement in on patch but that would mean that the code will not be build when bisecting without the latest patch adding the per architecture code. I'm fine with the both options. Andrew, what do you think would be the best here ? Thanks, Laurent. > > All the comments here definitely hold for the arm64 patches that you > plan to include with the next update. > > Thanks, > Punit > >> >> # >> # Arch settings >
Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: > On 08/05/2018 13:04, Punit Agrawal wrote: >> Hi Laurent, >> >> Laurent Dufour <ldufour@linux.vnet.ibm.com> writes: >> >>> Set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT which turns on the >>> Speculative Page Fault handler when building for 64bit. >>> >>> Cc: Thomas Gleixner <tglx@linutronix.de> >>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> >>> --- >>> arch/x86/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>> index d8983df5a2bc..ebdeb48e4a4a 100644 >>> --- a/arch/x86/Kconfig >>> +++ b/arch/x86/Kconfig >>> @@ -30,6 +30,7 @@ config X86_64 >>> select MODULES_USE_ELF_RELA >>> select X86_DEV_DMA_OPS >>> select ARCH_HAS_SYSCALL_WRAPPER >>> + select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT >> >> I'd suggest merging this patch with the one making changes to the >> architectural fault handler towards the end of the series. >> >> The Kconfig change is closely tied to the architectural support for SPF >> and makes sense to be in a single patch. >> >> If there's a good reason to keep them as separate patches, please move >> the architecture Kconfig changes after the patch adding fault handler >> changes. >> >> It's better to enable the feature once the core infrastructure is merged >> rather than at the beginning of the series to avoid potential bad >> fallout from incomplete functionality during bisection. > > Indeed bisection was the reason why Andrew asked me to push the configuration > enablement on top of the series (https://lkml.org/lkml/2017/10/10/1229). The config options have gone through another round of splitting (between core and architecture) since that comment. I agree that it still makes sense to define the core config - CONFIG_SPECULATIVE_PAGE_FAULT early on. Just to clarify, my suggestion was to only move the architecture configs further down. > > I also think it would be better to have the architecture enablement in on patch > but that would mean that the code will not be build when bisecting without the > latest patch adding the per architecture code. I don't see that as a problem. But if I'm in the minority, I am OK with leaving things as they are as well. Thanks, Punit
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index d8983df5a2bc..ebdeb48e4a4a 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -30,6 +30,7 @@ config X86_64 select MODULES_USE_ELF_RELA select X86_DEV_DMA_OPS select ARCH_HAS_SYSCALL_WRAPPER + select ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT # # Arch settings
Set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT which turns on the Speculative Page Fault handler when building for 64bit. Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> --- arch/x86/Kconfig | 1 + 1 file changed, 1 insertion(+)