Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote: > Hi Arnd, > > On Tue, Dec 19, 2017 at 12:39:33PM +0100, Arnd Bergmann wrote: >> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h >> index 5d595cfdb2c4..66cfdad68f7e 100644 >> --- a/include/linux/compiler-gcc.h >> +++ b/include/linux/compiler-gcc.h >> @@ -205,6 +205,15 @@ >> #endif >> >> /* >> + * calling noreturn functions, __builtin_unreachable() and __builtin_trap() >> + * confuse the stack allocation in gcc, leading to overly large stack >> + * frames, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365 >> + * >> + * Adding an empty inline assembly before it works around the problem >> + */ >> +#define barrier_before_unreachable() asm volatile("") >> + >> +/* >> * Mark a position in code as unreachable. This can be used to >> * suppress control flow warnings after asm blocks that transfer >> * control elsewhere. >> @@ -214,7 +223,11 @@ >> * unreleased. Really, we need to have autoconf for the kernel. >> */ >> #define unreachable() \ >> - do { annotate_unreachable(); __builtin_unreachable(); } while (0) >> + do {\ >> + annotate_unreachable(); \ >> + barrier_before_unreachable(); \ >> + __builtin_unreachable();\ >> + } while (0) > > Unfortunately this breaks microMIPS builds (e.g. MIPS > micro32r2_defconfig and micro32r2el_defconfig) on gcc 7.2, due to the > lack of .insn in the asm volatile. Because of the > __builtin_unreachable() there is no code following it. Without the empty > asm the compiler will apparently put the .insn there automatically, but > with the empty asm it doesn't. Therefore the assembler won't treat an > immediately preceeding label as pointing at 16-bit microMIPS > instructions which need the ISA bit set, i.e. bit 0 of the address. > This causes assembler errors since the branch target is treated as a > different ISA mode: > > arch/mips/mm/dma-default.s:3265: Error: branch to a symbol in another ISA mode > arch/mips/mm/dma-default.s:5027: Error: branch to a symbol in another ISA mode Ok, I see. > Due to a compiler bug on gcc 4.9.2 -> somewhere before 7.2, Paul > submitted these patches a while back: > https://patchwork.linux-mips.org/patch/13360/ > https://patchwork.linux-mips.org/patch/13361/ > > Your patch (suitably fixed for microMIPS) would I imagine fix that issue > too (it certainly fixes the resulting link error on microMIPS builds > with an old toolchain). > > Before I forward port those patches to add .insn for MIPS, is that sort > of approach (an arch specific asm/compiler-gcc.h to allow MIPS to > override barrier_before_unreachable()) an acceptable fix? That sounds fine to me. However, I would suggest making that asm/compiler.h instead of asm/compiler-gcc.h, so we can also use the same file to include workarounds for clang if needed. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
Remove the additional define HAVE_PTE_SPECIAL and rely directly on CONFIG_ARCH_HAS_PTE_SPECIAL. There is no functional change introduced by this patch Signed-off-by: Laurent Dufour --- mm/memory.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 96910c625daa..7f7dc7b2a341 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -817,17 +817,12 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr, * PFNMAP mappings in order to support COWable mappings. * */ -#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL -# define HAVE_PTE_SPECIAL 1 -#else -# define HAVE_PTE_SPECIAL 0 -#endif struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte, bool with_public_device) { unsigned long pfn = pte_pfn(pte); - if (HAVE_PTE_SPECIAL) { + if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) { if (likely(!pte_special(pte))) goto check_pfn; if (vma->vm_ops && vma->vm_ops->find_special_page) @@ -862,7 +857,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, return NULL; } - /* !HAVE_PTE_SPECIAL case follows: */ + /* !CONFIG_ARCH_HAS_PTE_SPECIAL case follows: */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; -check_pfn: + +check_pfn: __maybe_unused if (unlikely(pfn > highest_memmap_pfn)) { print_bad_pte(vma, addr, pte, NULL); return NULL; @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, * NOTE! We still have PageReserved() pages in the page tables. * eg. VDSO mappings can cause them to exist. */ -out: +out: __maybe_unused return pfn_to_page(pfn); } @@ -904,7 +900,7 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr, /* * There is no pmd_special() but there may be special pmds, e.g. * in a direct-access (dax) mapping, so let's just replicate the -* !HAVE_PTE_SPECIAL case from vm_normal_page() here. +* !CONFIG_ARCH_HAS_PTE_SPECIAL case from vm_normal_page() here. */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -1933,7 +1929,8 @@ static int __vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr, * than insert_pfn). If a zero_pfn were inserted into a VM_MIXEDMAP * without pte special, it would there be refcounted as a normal page. */ - if (!HAVE_PTE_SPECIAL && !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) { + if (!IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && + !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) { struct page *page; /* -- 2.7.4 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v3 0/2] move __HAVE_ARCH_PTE_SPECIAL in Kconfig
The per architecture __HAVE_ARCH_PTE_SPECIAL is defined statically in the per architecture header files. This doesn't allow to make other configuration dependent on it. The first patch of this series is replacing __HAVE_ARCH_PTE_SPECIAL by CONFIG_ARCH_HAS_PTE_SPECIAL defined into the Kconfig files, setting it automatically when architectures was already setting it in header file. The second patch is removing the odd define HAVE_PTE_SPECIAL which is a duplicate of CONFIG_ARCH_HAS_PTE_SPECIAL. There is no functional change introduced by this series. -- Changes since v2: * remove __HAVE_ARCH_PTE_SPECIAL in arch/riscv/include/asm/pgtable-bits.h * use IS_ENABLED() instead of #ifdef blocks in patch 2 Laurent Dufour (2): mm: introduce ARCH_HAS_PTE_SPECIAL mm: remove odd HAVE_PTE_SPECIAL .../features/vm/pte_special/arch-support.txt | 2 +- arch/arc/Kconfig | 1 + arch/arc/include/asm/pgtable.h| 2 -- arch/arm/Kconfig | 1 + arch/arm/include/asm/pgtable-3level.h | 1 - arch/arm64/Kconfig| 1 + arch/arm64/include/asm/pgtable.h | 2 -- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/book3s/64/pgtable.h | 3 --- arch/powerpc/include/asm/pte-common.h | 3 --- arch/riscv/Kconfig| 1 + arch/riscv/include/asm/pgtable-bits.h | 3 --- arch/s390/Kconfig | 1 + arch/s390/include/asm/pgtable.h | 1 - arch/sh/Kconfig | 1 + arch/sh/include/asm/pgtable.h | 2 -- arch/sparc/Kconfig| 1 + arch/sparc/include/asm/pgtable_64.h | 3 --- arch/x86/Kconfig | 1 + arch/x86/include/asm/pgtable_types.h | 1 - include/linux/pfn_t.h | 4 ++-- mm/Kconfig| 3 +++ mm/gup.c | 4 ++-- mm/memory.c | 19 --- 24 files changed, 25 insertions(+), 37 deletions(-) -- 2.7.4 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v3 1/2] mm: introduce ARCH_HAS_PTE_SPECIAL
Currently the PTE special supports is turned on in per architecture header files. Most of the time, it is defined in arch/*/include/asm/pgtable.h depending or not on some other per architecture static definition. This patch introduce a new configuration variable to manage this directly in the Kconfig files. It would later replace __HAVE_ARCH_PTE_SPECIAL. Here notes for some architecture where the definition of __HAVE_ARCH_PTE_SPECIAL is not obvious: arm __HAVE_ARCH_PTE_SPECIAL which is currently defined in arch/arm/include/asm/pgtable-3level.h which is included by arch/arm/include/asm/pgtable.h when CONFIG_ARM_LPAE is set. So select ARCH_HAS_PTE_SPECIAL if ARM_LPAE. powerpc __HAVE_ARCH_PTE_SPECIAL is defined in 2 files: - arch/powerpc/include/asm/book3s/64/pgtable.h - arch/powerpc/include/asm/pte-common.h The first one is included if (PPC_BOOK3S & PPC64) while the second is included in all the other cases. So select ARCH_HAS_PTE_SPECIAL all the time. sparc: __HAVE_ARCH_PTE_SPECIAL is defined if defined(__sparc__) && defined(__arch64__) which are defined through the compiler in sparc/Makefile if !SPARC32 which I assume to be if SPARC64. So select ARCH_HAS_PTE_SPECIAL if SPARC64 There is no functional change introduced by this patch. Suggested-by: Jerome Glisse Reviewed-by: Jerome Glisse Acked-by: David Rientjes Signed-off-by: Laurent Dufour --- Documentation/features/vm/pte_special/arch-support.txt | 2 +- arch/arc/Kconfig | 1 + arch/arc/include/asm/pgtable.h | 2 -- arch/arm/Kconfig | 1 + arch/arm/include/asm/pgtable-3level.h | 1 - arch/arm64/Kconfig | 1 + arch/arm64/include/asm/pgtable.h | 2 -- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/book3s/64/pgtable.h | 3 --- arch/powerpc/include/asm/pte-common.h | 3 --- arch/riscv/Kconfig | 1 + arch/riscv/include/asm/pgtable-bits.h | 3 --- arch/s390/Kconfig | 1 + arch/s390/include/asm/pgtable.h| 1 - arch/sh/Kconfig| 1 + arch/sh/include/asm/pgtable.h | 2 -- arch/sparc/Kconfig | 1 + arch/sparc/include/asm/pgtable_64.h| 3 --- arch/x86/Kconfig | 1 + arch/x86/include/asm/pgtable_types.h | 1 - include/linux/pfn_t.h | 4 ++-- mm/Kconfig | 3 +++ mm/gup.c | 4 ++-- mm/memory.c| 2 +- 24 files changed, 18 insertions(+), 27 deletions(-) diff --git a/Documentation/features/vm/pte_special/arch-support.txt b/Documentation/features/vm/pte_special/arch-support.txt index 055004f467d2..cd05924ea875 100644 --- a/Documentation/features/vm/pte_special/arch-support.txt +++ b/Documentation/features/vm/pte_special/arch-support.txt @@ -1,6 +1,6 @@ # # Feature name: pte_special -# Kconfig: __HAVE_ARCH_PTE_SPECIAL +# Kconfig: ARCH_HAS_PTE_SPECIAL # description: arch supports the pte_special()/pte_mkspecial() VM APIs # --- diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig index d76bf4a83740..8516e2b0239a 100644 --- a/arch/arc/Kconfig +++ b/arch/arc/Kconfig @@ -44,6 +44,7 @@ config ARC select HAVE_GENERIC_DMA_COHERENT select HAVE_KERNEL_GZIP select HAVE_KERNEL_LZMA + select ARCH_HAS_PTE_SPECIAL config MIGHT_HAVE_PCI bool diff --git a/arch/arc/include/asm/pgtable.h b/arch/arc/include/asm/pgtable.h index 08fe33830d4b..8ec5599a0957 100644 --- a/arch/arc/include/asm/pgtable.h +++ b/arch/arc/include/asm/pgtable.h @@ -320,8 +320,6 @@ PTE_BIT_FUNC(mkexec,|= (_PAGE_EXECUTE)); PTE_BIT_FUNC(mkspecial,|= (_PAGE_SPECIAL)); PTE_BIT_FUNC(mkhuge, |= (_PAGE_HW_SZ)); -#define __HAVE_ARCH_PTE_SPECIAL - static inline pte_t pte_modify(pte_t pte, pgprot_t newprot) { return __pte((pte_val(pte) & _PAGE_CHG_MASK) | pgprot_val(newprot)); diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index a7f8e7f4b88f..c088c851b235 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -8,6 +8,7 @@ config ARM select ARCH_HAS_DEVMEM_IS_ALLOWED select ARCH_HAS_ELF_RANDOMIZE select ARCH_HAS_FORTIFY_SOURCE + select ARCH_HAS_PTE_SPECIAL if ARM_LPAE select ARCH_HAS_SET_MEMORY select ARCH_HAS_PHYS_TO_DMA select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h index 2a4836087358..6d50a11d7793 100644 --- a/arch/arm/include/asm
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On Wed 11-04-18 10:03:36, Laurent Dufour wrote: > @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, > unsigned long addr, > > if (is_zero_pfn(pfn)) > return NULL; > -check_pfn: > + > +check_pfn: __maybe_unused > if (unlikely(pfn > highest_memmap_pfn)) { > print_bad_pte(vma, addr, pte, NULL); > return NULL; > @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, > unsigned long addr, >* NOTE! We still have PageReserved() pages in the page tables. >* eg. VDSO mappings can cause them to exist. >*/ > -out: > +out: __maybe_unused > return pfn_to_page(pfn); Why do we need this ugliness all of the sudden? -- Michal Hocko SUSE Labs ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 1/2] mm: introduce ARCH_HAS_PTE_SPECIAL
On Wed 11-04-18 10:03:35, Laurent Dufour wrote: > Currently the PTE special supports is turned on in per architecture header > files. Most of the time, it is defined in arch/*/include/asm/pgtable.h > depending or not on some other per architecture static definition. > > This patch introduce a new configuration variable to manage this directly > in the Kconfig files. It would later replace __HAVE_ARCH_PTE_SPECIAL. > > Here notes for some architecture where the definition of > __HAVE_ARCH_PTE_SPECIAL is not obvious: > > arm > __HAVE_ARCH_PTE_SPECIAL which is currently defined in > arch/arm/include/asm/pgtable-3level.h which is included by > arch/arm/include/asm/pgtable.h when CONFIG_ARM_LPAE is set. > So select ARCH_HAS_PTE_SPECIAL if ARM_LPAE. > > powerpc > __HAVE_ARCH_PTE_SPECIAL is defined in 2 files: > - arch/powerpc/include/asm/book3s/64/pgtable.h > - arch/powerpc/include/asm/pte-common.h > The first one is included if (PPC_BOOK3S & PPC64) while the second is > included in all the other cases. > So select ARCH_HAS_PTE_SPECIAL all the time. > > sparc: > __HAVE_ARCH_PTE_SPECIAL is defined if defined(__sparc__) && > defined(__arch64__) which are defined through the compiler in > sparc/Makefile if !SPARC32 which I assume to be if SPARC64. > So select ARCH_HAS_PTE_SPECIAL if SPARC64 > > There is no functional change introduced by this patch. > > Suggested-by: Jerome Glisse > Reviewed-by: Jerome Glisse > Acked-by: David Rientjes > Signed-off-by: Laurent Dufour Looks good to me. I have checked x86 and the generic code and it looks good to me. Anyway arch maintainers really have to double check this. -- Michal Hocko SUSE Labs ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On 11/04/2018 10:33, Michal Hocko wrote: > On Wed 11-04-18 10:03:36, Laurent Dufour wrote: >> @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, >> unsigned long addr, >> >> if (is_zero_pfn(pfn)) >> return NULL; >> -check_pfn: >> + >> +check_pfn: __maybe_unused >> if (unlikely(pfn > highest_memmap_pfn)) { >> print_bad_pte(vma, addr, pte, NULL); >> return NULL; >> @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, >> unsigned long addr, >> * NOTE! We still have PageReserved() pages in the page tables. >> * eg. VDSO mappings can cause them to exist. >> */ >> -out: >> +out: __maybe_unused >> return pfn_to_page(pfn); > > Why do we need this ugliness all of the sudden? Indeed the compiler doesn't complaint but in theory it should since these labels are not used depending on CONFIG_ARCH_HAS_PTE_SPECIAL. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On Wed 11-04-18 10:41:23, Laurent Dufour wrote: > On 11/04/2018 10:33, Michal Hocko wrote: > > On Wed 11-04-18 10:03:36, Laurent Dufour wrote: > >> @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct > >> *vma, unsigned long addr, > >> > >>if (is_zero_pfn(pfn)) > >>return NULL; > >> -check_pfn: > >> + > >> +check_pfn: __maybe_unused > >>if (unlikely(pfn > highest_memmap_pfn)) { > >>print_bad_pte(vma, addr, pte, NULL); > >>return NULL; > >> @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct > >> *vma, unsigned long addr, > >> * NOTE! We still have PageReserved() pages in the page tables. > >> * eg. VDSO mappings can cause them to exist. > >> */ > >> -out: > >> +out: __maybe_unused > >>return pfn_to_page(pfn); > > > > Why do we need this ugliness all of the sudden? > Indeed the compiler doesn't complaint but in theory it should since these > labels are not used depending on CONFIG_ARCH_HAS_PTE_SPECIAL. Well, such a warning would be quite pointless so I would rather not make the code ugly. The value of unused label is quite questionable to start with... -- Michal Hocko SUSE Labs ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
Le 11/04/2018 à 10:41, Laurent Dufour a écrit : On 11/04/2018 10:33, Michal Hocko wrote: On Wed 11-04-18 10:03:36, Laurent Dufour wrote: @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; -check_pfn: + +check_pfn: __maybe_unused if (unlikely(pfn > highest_memmap_pfn)) { print_bad_pte(vma, addr, pte, NULL); return NULL; @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, * NOTE! We still have PageReserved() pages in the page tables. * eg. VDSO mappings can cause them to exist. */ -out: +out: __maybe_unused return pfn_to_page(pfn); Why do we need this ugliness all of the sudden? Indeed the compiler doesn't complaint but in theory it should since these labels are not used depending on CONFIG_ARCH_HAS_PTE_SPECIAL. Why should it complain ? Regards Christophe ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On 11/04/2018 10:58, Christophe LEROY wrote: > > > Le 11/04/2018 à 10:03, Laurent Dufour a écrit : >> Remove the additional define HAVE_PTE_SPECIAL and rely directly on >> CONFIG_ARCH_HAS_PTE_SPECIAL. >> >> There is no functional change introduced by this patch >> >> Signed-off-by: Laurent Dufour >> --- >> mm/memory.c | 19 --- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index 96910c625daa..7f7dc7b2a341 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -817,17 +817,12 @@ static void print_bad_pte(struct vm_area_struct *vma, >> unsigned long addr, >> * PFNMAP mappings in order to support COWable mappings. >> * >> */ >> -#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL >> -# define HAVE_PTE_SPECIAL 1 >> -#else >> -# define HAVE_PTE_SPECIAL 0 >> -#endif >> struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long >> addr, >> pte_t pte, bool with_public_device) >> { >> unsigned long pfn = pte_pfn(pte); >> - if (HAVE_PTE_SPECIAL) { >> + if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) { >> if (likely(!pte_special(pte))) >> goto check_pfn; >> if (vma->vm_ops && vma->vm_ops->find_special_page) >> @@ -862,7 +857,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, >> unsigned long addr, >> return NULL; >> } >> - /* !HAVE_PTE_SPECIAL case follows: */ >> + /* !CONFIG_ARCH_HAS_PTE_SPECIAL case follows: */ >> if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { >> if (vma->vm_flags & VM_MIXEDMAP) { >> @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, >> unsigned long addr, >> if (is_zero_pfn(pfn)) >> return NULL; >> -check_pfn: >> + >> +check_pfn: __maybe_unused > > See below > >> if (unlikely(pfn > highest_memmap_pfn)) { >> print_bad_pte(vma, addr, pte, NULL); >> return NULL; >> @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, >> unsigned long addr, >> * NOTE! We still have PageReserved() pages in the page tables. >> * eg. VDSO mappings can cause them to exist. >> */ >> -out: >> +out: __maybe_unused > > Why do you need that change ? > > There is no reason for the compiler to complain. It would complain if the goto > was within a #ifdef, but all the purpose of using IS_ENABLED() is to allow the > compiler to properly handle all possible cases. That's all the force of > IS_ENABLED() compared to ifdefs, and that the reason why they are plebicited, > ref Linux Codying style for a detailed explanation. Fair enough. Should I submit a v4 just to remove these so ugly __maybe_unused ? ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
Le 11/04/2018 à 10:03, Laurent Dufour a écrit : Remove the additional define HAVE_PTE_SPECIAL and rely directly on CONFIG_ARCH_HAS_PTE_SPECIAL. There is no functional change introduced by this patch Signed-off-by: Laurent Dufour --- mm/memory.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 96910c625daa..7f7dc7b2a341 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -817,17 +817,12 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr, * PFNMAP mappings in order to support COWable mappings. * */ -#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL -# define HAVE_PTE_SPECIAL 1 -#else -# define HAVE_PTE_SPECIAL 0 -#endif struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte, bool with_public_device) { unsigned long pfn = pte_pfn(pte); - if (HAVE_PTE_SPECIAL) { + if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) { if (likely(!pte_special(pte))) goto check_pfn; if (vma->vm_ops && vma->vm_ops->find_special_page) @@ -862,7 +857,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, return NULL; } - /* !HAVE_PTE_SPECIAL case follows: */ + /* !CONFIG_ARCH_HAS_PTE_SPECIAL case follows: */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; -check_pfn: + +check_pfn: __maybe_unused See below if (unlikely(pfn > highest_memmap_pfn)) { print_bad_pte(vma, addr, pte, NULL); return NULL; @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, * NOTE! We still have PageReserved() pages in the page tables. * eg. VDSO mappings can cause them to exist. */ -out: +out: __maybe_unused Why do you need that change ? There is no reason for the compiler to complain. It would complain if the goto was within a #ifdef, but all the purpose of using IS_ENABLED() is to allow the compiler to properly handle all possible cases. That's all the force of IS_ENABLED() compared to ifdefs, and that the reason why they are plebicited, ref Linux Codying style for a detailed explanation. Christophe return pfn_to_page(pfn); } @@ -904,7 +900,7 @@ struct page *vm_normal_page_pmd(struct vm_area_struct *vma, unsigned long addr, /* * There is no pmd_special() but there may be special pmds, e.g. * in a direct-access (dax) mapping, so let's just replicate the -* !HAVE_PTE_SPECIAL case from vm_normal_page() here. +* !CONFIG_ARCH_HAS_PTE_SPECIAL case from vm_normal_page() here. */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -1933,7 +1929,8 @@ static int __vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr, * than insert_pfn). If a zero_pfn were inserted into a VM_MIXEDMAP * without pte special, it would there be refcounted as a normal page. */ - if (!HAVE_PTE_SPECIAL && !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) { + if (!IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL) && + !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) { struct page *page; /* ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
Le 11/04/2018 à 11:03, Laurent Dufour a écrit : On 11/04/2018 10:58, Christophe LEROY wrote: Le 11/04/2018 à 10:03, Laurent Dufour a écrit : Remove the additional define HAVE_PTE_SPECIAL and rely directly on CONFIG_ARCH_HAS_PTE_SPECIAL. There is no functional change introduced by this patch Signed-off-by: Laurent Dufour --- mm/memory.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 96910c625daa..7f7dc7b2a341 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -817,17 +817,12 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr, * PFNMAP mappings in order to support COWable mappings. * */ -#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL -# define HAVE_PTE_SPECIAL 1 -#else -# define HAVE_PTE_SPECIAL 0 -#endif struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte, bool with_public_device) { unsigned long pfn = pte_pfn(pte); - if (HAVE_PTE_SPECIAL) { + if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) { if (likely(!pte_special(pte))) goto check_pfn; if (vma->vm_ops && vma->vm_ops->find_special_page) @@ -862,7 +857,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, return NULL; } - /* !HAVE_PTE_SPECIAL case follows: */ + /* !CONFIG_ARCH_HAS_PTE_SPECIAL case follows: */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; -check_pfn: + +check_pfn: __maybe_unused See below if (unlikely(pfn > highest_memmap_pfn)) { print_bad_pte(vma, addr, pte, NULL); return NULL; @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, * NOTE! We still have PageReserved() pages in the page tables. * eg. VDSO mappings can cause them to exist. */ -out: +out: __maybe_unused Why do you need that change ? There is no reason for the compiler to complain. It would complain if the goto was within a #ifdef, but all the purpose of using IS_ENABLED() is to allow the compiler to properly handle all possible cases. That's all the force of IS_ENABLED() compared to ifdefs, and that the reason why they are plebicited, ref Linux Codying style for a detailed explanation. Fair enough. Should I submit a v4 just to remove these so ugly __maybe_unused ? Most likely, unless the mm maintainer agrees to remove them by himself when applying your patch ? Christophe ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 09:30:56AM +0200, Arnd Bergmann wrote: > On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote: > > Before I forward port those patches to add .insn for MIPS, is that sort > > of approach (an arch specific asm/compiler-gcc.h to allow MIPS to > > override barrier_before_unreachable()) an acceptable fix? > > That sounds fine to me. However, I would suggest making that > asm/compiler.h instead of asm/compiler-gcc.h, so we can also > use the same file to include workarounds for clang if needed. Yes, though there are a few asm/compiler.h's already, and the alpha one includes linux/compiler.h before undefining inline, so seems to have its own specific purpose... Cheers James signature.asc Description: Digital signature ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 11:54 AM, James Hogan wrote: > On Wed, Apr 11, 2018 at 09:30:56AM +0200, Arnd Bergmann wrote: >> On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote: >> > Before I forward port those patches to add .insn for MIPS, is that sort >> > of approach (an arch specific asm/compiler-gcc.h to allow MIPS to >> > override barrier_before_unreachable()) an acceptable fix? >> >> That sounds fine to me. However, I would suggest making that >> asm/compiler.h instead of asm/compiler-gcc.h, so we can also >> use the same file to include workarounds for clang if needed. > > Yes, though there are a few asm/compiler.h's already, and the alpha one > includes linux/compiler.h before undefining inline, so seems to have its > own specific purpose... Interesting. For the other ones, including asm/compiler.h from linux/compiler.h seems appropriate though, so the question would be what to do with the alpha case. I think we can simply remove that header file and replace it with this patch: diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index b2022885ced8..5502404f54cd 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -81,6 +81,9 @@ config PGTABLE_LEVELS int default 3 +config OPTIMIZE_INLINING + def_bool y + source "init/Kconfig" source "kernel/Kconfig.freezer" which should have the same effect. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 12:08:51PM +0200, Arnd Bergmann wrote: > On Wed, Apr 11, 2018 at 11:54 AM, James Hogan wrote: > > On Wed, Apr 11, 2018 at 09:30:56AM +0200, Arnd Bergmann wrote: > >> On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote: > >> > Before I forward port those patches to add .insn for MIPS, is that sort > >> > of approach (an arch specific asm/compiler-gcc.h to allow MIPS to > >> > override barrier_before_unreachable()) an acceptable fix? > >> > >> That sounds fine to me. However, I would suggest making that > >> asm/compiler.h instead of asm/compiler-gcc.h, so we can also > >> use the same file to include workarounds for clang if needed. > > > > Yes, though there are a few asm/compiler.h's already, and the alpha one > > includes linux/compiler.h before undefining inline, so seems to have its > > own specific purpose... > > Interesting. For the other ones, including asm/compiler.h from > linux/compiler.h > seems appropriate though, so the question would be what to do with the > alpha case. I think we can simply remove that header file and replace > it with this patch: > > diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig > index b2022885ced8..5502404f54cd 100644 > --- a/arch/alpha/Kconfig > +++ b/arch/alpha/Kconfig > @@ -81,6 +81,9 @@ config PGTABLE_LEVELS > int > default 3 > > +config OPTIMIZE_INLINING > + def_bool y > + > source "init/Kconfig" > source "kernel/Kconfig.freezer" > > which should have the same effect. Hmm yes, and I suppose alpha would need ARCH_SUPPORTS_OPTIMIZED_INLINING too. I'll give it a try. Cheers James signature.asc Description: Digital signature ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On 11/04/2018 11:09, Christophe LEROY wrote: > > > Le 11/04/2018 à 11:03, Laurent Dufour a écrit : >> >> >> On 11/04/2018 10:58, Christophe LEROY wrote: >>> >>> >>> Le 11/04/2018 à 10:03, Laurent Dufour a écrit : Remove the additional define HAVE_PTE_SPECIAL and rely directly on CONFIG_ARCH_HAS_PTE_SPECIAL. There is no functional change introduced by this patch Signed-off-by: Laurent Dufour --- mm/memory.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 96910c625daa..7f7dc7b2a341 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -817,17 +817,12 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr, * PFNMAP mappings in order to support COWable mappings. * */ -#ifdef CONFIG_ARCH_HAS_PTE_SPECIAL -# define HAVE_PTE_SPECIAL 1 -#else -# define HAVE_PTE_SPECIAL 0 -#endif struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, pte_t pte, bool with_public_device) { unsigned long pfn = pte_pfn(pte); - if (HAVE_PTE_SPECIAL) { + if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) { if (likely(!pte_special(pte))) goto check_pfn; if (vma->vm_ops && vma->vm_ops->find_special_page) @@ -862,7 +857,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, return NULL; } - /* !HAVE_PTE_SPECIAL case follows: */ + /* !CONFIG_ARCH_HAS_PTE_SPECIAL case follows: */ if (unlikely(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP))) { if (vma->vm_flags & VM_MIXEDMAP) { @@ -881,7 +876,8 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; -check_pfn: + +check_pfn: __maybe_unused >>> >>> See below >>> if (unlikely(pfn > highest_memmap_pfn)) { print_bad_pte(vma, addr, pte, NULL); return NULL; @@ -891,7 +887,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr, * NOTE! We still have PageReserved() pages in the page tables. * eg. VDSO mappings can cause them to exist. */ -out: +out: __maybe_unused >>> >>> Why do you need that change ? >>> >>> There is no reason for the compiler to complain. It would complain if the >>> goto >>> was within a #ifdef, but all the purpose of using IS_ENABLED() is to allow >>> the >>> compiler to properly handle all possible cases. That's all the force of >>> IS_ENABLED() compared to ifdefs, and that the reason why they are >>> plebicited, >>> ref Linux Codying style for a detailed explanation. >> >> Fair enough. >> >> Should I submit a v4 just to remove these so ugly __maybe_unused ? >> > > Most likely, unless the mm maintainer agrees to remove them by himself when > applying your patch ? That was my point. Andrew, should I send a v4 or could you wipe the 2 __maybe_unsued when applying the patch ? Thanks, Laurent. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] mm: remove odd HAVE_PTE_SPECIAL
On Wed 11-04-18 12:32:07, Laurent Dufour wrote: [...] > Andrew, should I send a v4 or could you wipe the 2 __maybe_unsued when > applying > the patch ? A follow $patch-fix should be better rather than post this again and spam people with more emails. -- Michal Hocko SUSE Labs ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc