2023-08-02 07:24:05

by Minda Chen

[permalink] [raw]
Subject: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

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 <[email protected]>
---
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



2023-08-02 09:13:01

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

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 <[email protected]>
> >> >
> >> > 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 <[email protected]>
> >> >
> >> > 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

2023-08-02 09:16:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

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 <[email protected]>
> > >> >
> > >> > 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 <[email protected]>
> > >> >
> > >> > 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?


Attachments:
(No filename) (2.47 kB)
signature.asc (235.00 B)
Download all attachments

2023-08-02 10:19:19

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

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.


Attachments:
(No filename) (1.58 kB)
signature.asc (235.00 B)
Download all attachments

2023-08-02 10:21:47

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

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 <[email protected]>
> > > >> >
> > > >> > 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 <[email protected]>
> > > >> >
> > > >> > 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.


Subject: Re: [PATCH v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause

Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:

On Wed, 2 Aug 2023 14:42:15 +0800 you 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. 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.
>
> [...]

Here is the summary with links:
- [v1] riscv: Using TOOLCHAIN_HAS_ZIHINTPAUSE marco replace zihintpause
https://git.kernel.org/riscv/c/a09560a7b160

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html