| Message ID | 20230802064215.31111-1-minda.chen@starfivetech.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a09560a7b160381234a6f3a37a74968f9c75f666 |
| Headers | show |
| Series | [v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 2810 this patch: 2810 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 15878 this patch: 15878 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Hey Minda, On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: > Actually it is a part of Conor's > commit aae538cd03bc ("riscv: fix detection of toolchain > Zihintpause support"). > It is looks like a merge issue. Yup, spot on. > Samuel's > commit 0b1d60d6dd9e ("riscv: Fix build with > CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and > revert to __riscv_zihintpause. So this patch can fix it. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> Did you actually manage to trigger this, or was this by inspection? clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's what the clang-built-linux CI uses to test the LTS kernels from before LLVM's IAS was supported for RISC-V. Seemingly all that needs to be satisfied there is that zihintpause doesn't appear in -march so this has gone unnoticed. Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > --- > arch/riscv/include/asm/vdso/processor.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h > index 14f5d27783b8..96b65a5396df 100644 > --- a/arch/riscv/include/asm/vdso/processor.h > +++ b/arch/riscv/include/asm/vdso/processor.h > @@ -14,7 +14,7 @@ static inline void cpu_relax(void) > __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy)); > #endif > > -#ifdef __riscv_zihintpause > +#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE > /* > * Reduce instruction retirement. > * This assumes the PC changes. > -- > 2.17.1 >
On 2023/8/2 14:54, Conor Dooley wrote: > Hey Minda, > > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: >> Actually it is a part of Conor's >> commit aae538cd03bc ("riscv: fix detection of toolchain >> Zihintpause support"). >> It is looks like a merge issue. > > Yup, spot on. > >> Samuel's >> commit 0b1d60d6dd9e ("riscv: Fix build with >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and >> revert to __riscv_zihintpause. So this patch can fix it. >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > > Did you actually manage to trigger this, or was this by inspection? > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's > what the clang-built-linux CI uses to test the LTS kernels from before > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be > satisfied there is that zihintpause doesn't appear in -march so this has > gone unnoticed. > > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > Thanks, > Conor. > Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log of processor.h found this issue. >> --- >> arch/riscv/include/asm/vdso/processor.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h >> index 14f5d27783b8..96b65a5396df 100644 >> --- a/arch/riscv/include/asm/vdso/processor.h >> +++ b/arch/riscv/include/asm/vdso/processor.h >> @@ -14,7 +14,7 @@ static inline void cpu_relax(void) >> __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy)); >> #endif >> >> -#ifdef __riscv_zihintpause >> +#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE >> /* >> * Reduce instruction retirement. >> * This assumes the PC changes. >> -- >> 2.17.1 >>
On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: > > > On 2023/8/2 14:54, Conor Dooley wrote: > > Hey Minda, > > > > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: > >> Actually it is a part of Conor's > >> commit aae538cd03bc ("riscv: fix detection of toolchain > >> Zihintpause support"). > >> It is looks like a merge issue. > > > > Yup, spot on. > > > >> Samuel's > >> commit 0b1d60d6dd9e ("riscv: Fix build with > >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and > >> revert to __riscv_zihintpause. So this patch can fix it. > >> > >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > > > > Did you actually manage to trigger this, or was this by inspection? > > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's > > what the clang-built-linux CI uses to test the LTS kernels from before > > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be > > satisfied there is that zihintpause doesn't appear in -march so this has > > gone unnoticed. > > > > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > > Thanks, > > Conor. > > > Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax > cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log > of processor.h found this issue. That doesn't look like it is fixed in later stable kernels (we are at 6.1.42-rcN right now I think). It sounds we should ask Greg to backport 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") to 6.1. Does that make sense to you?
On 2023/8/2 15:48, Conor Dooley wrote: > On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: >> >> >> On 2023/8/2 14:54, Conor Dooley wrote: >> > Hey Minda, >> > >> > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: >> >> Actually it is a part of Conor's >> >> commit aae538cd03bc ("riscv: fix detection of toolchain >> >> Zihintpause support"). >> >> It is looks like a merge issue. >> > >> > Yup, spot on. >> > >> >> Samuel's >> >> commit 0b1d60d6dd9e ("riscv: Fix build with >> >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and >> >> revert to __riscv_zihintpause. So this patch can fix it. >> >> >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> >> > >> > Did you actually manage to trigger this, or was this by inspection? >> > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's >> > what the clang-built-linux CI uses to test the LTS kernels from before >> > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be >> > satisfied there is that zihintpause doesn't appear in -march so this has >> > gone unnoticed. >> > >> > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") >> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> > >> > Thanks, >> > Conor. >> > >> Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax >> cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log >> of processor.h found this issue. > > That doesn't look like it is fixed in later stable kernels (we are at > 6.1.42-rcN right now I think). It sounds we should ask Greg to backport > 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") > to 6.1. Does that make sense to you? Yes. 6.1 is lts kernel. Starfive will use this kernel for a long time. Thanks.
On Wed, Aug 02, 2023 at 04:17:51PM +0800, Minda Chen wrote: > > > On 2023/8/2 15:48, Conor Dooley wrote: > > On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: > >> > >> > >> On 2023/8/2 14:54, Conor Dooley wrote: > >> > Hey Minda, > >> > > >> > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: > >> >> Actually it is a part of Conor's > >> >> commit aae538cd03bc ("riscv: fix detection of toolchain > >> >> Zihintpause support"). > >> >> It is looks like a merge issue. > >> > > >> > Yup, spot on. > >> > > >> >> Samuel's > >> >> commit 0b1d60d6dd9e ("riscv: Fix build with > >> >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and > >> >> revert to __riscv_zihintpause. So this patch can fix it. > >> >> > >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > >> > > >> > Did you actually manage to trigger this, or was this by inspection? > >> > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's > >> > what the clang-built-linux CI uses to test the LTS kernels from before > >> > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be > >> > satisfied there is that zihintpause doesn't appear in -march so this has > >> > gone unnoticed. > >> > > >> > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") > >> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > >> > > >> > Thanks, > >> > Conor. > >> > > >> Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax > >> cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log > >> of processor.h found this issue. > > > > That doesn't look like it is fixed in later stable kernels (we are at > > 6.1.42-rcN right now I think). It sounds we should ask Greg to backport > > 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") > > to 6.1. Does that make sense to you? > Yes. 6.1 is lts kernel. Starfive will use this kernel for a long time. Thanks. What is preventing you from moving to a newer kernel version? All of your kernel changes are already properly merged into Linus's tree, right? thanks, greg k-h
On Wed, Aug 02, 2023 at 10:33:27AM +0200, Greg KH wrote: > On Wed, Aug 02, 2023 at 04:17:51PM +0800, Minda Chen wrote: > > > > > > On 2023/8/2 15:48, Conor Dooley wrote: > > > On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: > > >> > > >> > > >> On 2023/8/2 14:54, Conor Dooley wrote: > > >> > Hey Minda, > > >> > > > >> > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: > > >> >> Actually it is a part of Conor's > > >> >> commit aae538cd03bc ("riscv: fix detection of toolchain > > >> >> Zihintpause support"). > > >> >> It is looks like a merge issue. > > >> > > > >> > Yup, spot on. > > >> > > > >> >> Samuel's > > >> >> commit 0b1d60d6dd9e ("riscv: Fix build with > > >> >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and > > >> >> revert to __riscv_zihintpause. So this patch can fix it. > > >> >> > > >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > > >> > > > >> > Did you actually manage to trigger this, or was this by inspection? > > >> > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's > > >> > what the clang-built-linux CI uses to test the LTS kernels from before > > >> > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be > > >> > satisfied there is that zihintpause doesn't appear in -march so this has > > >> > gone unnoticed. > > >> > > > >> > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") > > >> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > >> > > > >> > Thanks, > > >> > Conor. > > >> > > > >> Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax > > >> cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log > > >> of processor.h found this issue. > > > > > > That doesn't look like it is fixed in later stable kernels (we are at > > > 6.1.42-rcN right now I think). It sounds we should ask Greg to backport > > > 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") > > > to 6.1. Does that make sense to you? > > Yes. 6.1 is lts kernel. Starfive will use this kernel for a long time. Thanks. > > What is preventing you from moving to a newer kernel version? All of > your kernel changes are already properly merged into Linus's tree, > right? Regardless of their reasons, "vdso.so call cpu_relax cause application core dump" is something that we should fix in stable kernels, no?
On Wed, Aug 02, 2023 at 09:52:45AM +0100, Conor Dooley wrote: > On Wed, Aug 02, 2023 at 10:33:27AM +0200, Greg KH wrote: > > On Wed, Aug 02, 2023 at 04:17:51PM +0800, Minda Chen wrote: > > > > > > > > > On 2023/8/2 15:48, Conor Dooley wrote: > > > > On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: > > > >> > > > >> > > > >> On 2023/8/2 14:54, Conor Dooley wrote: > > > >> > Hey Minda, > > > >> > > > > >> > On Wed, Aug 02, 2023 at 02:42:15PM +0800, Minda Chen wrote: > > > >> >> Actually it is a part of Conor's > > > >> >> commit aae538cd03bc ("riscv: fix detection of toolchain > > > >> >> Zihintpause support"). > > > >> >> It is looks like a merge issue. > > > >> > > > > >> > Yup, spot on. > > > >> > > > > >> >> Samuel's > > > >> >> commit 0b1d60d6dd9e ("riscv: Fix build with > > > >> >> CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and > > > >> >> revert to __riscv_zihintpause. So this patch can fix it. > > > >> >> > > > >> >> Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > > > >> > > > > >> > Did you actually manage to trigger this, or was this by inspection? > > > >> > clang-15 + binutils 2.35 was, IIRC, how we spotted this because that's > > > >> > what the clang-built-linux CI uses to test the LTS kernels from before > > > >> > LLVM's IAS was supported for RISC-V. Seemingly all that needs to be > > > >> > satisfied there is that zihintpause doesn't appear in -march so this has > > > >> > gone unnoticed. > > > >> > > > > >> > Fixes: 3c349eacc559 ("Merge patch "riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y"") > > > >> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > >> > > > > >> > Thanks, > > > >> > Conor. > > > >> > > > > >> Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax > > > >> cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log > > > >> of processor.h found this issue. > > > > > > > > That doesn't look like it is fixed in later stable kernels (we are at > > > > 6.1.42-rcN right now I think). It sounds we should ask Greg to backport > > > > 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") > > > > to 6.1. Does that make sense to you? > > > Yes. 6.1 is lts kernel. Starfive will use this kernel for a long time. Thanks. > > > > What is preventing you from moving to a newer kernel version? All of > > your kernel changes are already properly merged into Linus's tree, > > right? > > Regardless of their reasons, "vdso.so call cpu_relax cause application > core dump" is something that we should fix in stable kernels, no? Yes.
On Wed, Aug 02, 2023 at 11:42:59AM +0200, Greg KH wrote: > On Wed, Aug 02, 2023 at 09:52:45AM +0100, Conor Dooley wrote: > > On Wed, Aug 02, 2023 at 10:33:27AM +0200, Greg KH wrote: > > > On Wed, Aug 02, 2023 at 04:17:51PM +0800, Minda Chen wrote: > > > > On 2023/8/2 15:48, Conor Dooley wrote: > > > > > On Wed, Aug 02, 2023 at 03:32:15PM +0800, Minda Chen wrote: > > > > >> On 2023/8/2 14:54, Conor Dooley wrote: > > > > >> Thanks, Conor. I found this just by inspection. I found a issue that vdso.so call cpu_relax > > > > >> cause application core dump in kernel 6.1.31. I need Samuel'patch to fix this. And I search the log > > > > >> of processor.h found this issue. > > > > > > > > > > That doesn't look like it is fixed in later stable kernels (we are at > > > > > 6.1.42-rcN right now I think). It sounds we should ask Greg to backport > > > > > 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") > > > > > to 6.1. Does that make sense to you? > > > > Yes. 6.1 is lts kernel. Starfive will use this kernel for a long time. Thanks. > > > > > > What is preventing you from moving to a newer kernel version? All of > > > your kernel changes are already properly merged into Linus's tree, > > > right? > > > > Regardless of their reasons, "vdso.so call cpu_relax cause application > > core dump" is something that we should fix in stable kernels, no? > > Yes. It doesn't apply cleanly as a cherry-pick onto linux-6.1, so it'll need to be submitted. Maybe Minda can do that, since they've got an already tested version of the patch. Failing that, I will.
diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h index 14f5d27783b8..96b65a5396df 100644 --- a/arch/riscv/include/asm/vdso/processor.h +++ b/arch/riscv/include/asm/vdso/processor.h @@ -14,7 +14,7 @@ static inline void cpu_relax(void) __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy)); #endif -#ifdef __riscv_zihintpause +#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE /* * Reduce instruction retirement. * This assumes the PC changes.
Actually it is a part of Conor's commit aae538cd03bc ("riscv: fix detection of toolchain Zihintpause support"). It is looks like a merge issue. Samuel's commit 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y") do not base on Conor's commit and revert to __riscv_zihintpause. So this patch can fix it. Signed-off-by: Minda Chen <minda.chen@starfivetech.com> --- arch/riscv/include/asm/vdso/processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)