| Message ID | 20240717111716.157149-1-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [RFC] riscv: Allow to build only with LLVM >= 17.0.0 | expand |
On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote: > The following build failure happens when using LLVM < 17.0.0: > > kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets > > This is a known issue [1] so let's upgrade the minimal requirement for > LLVM to the version 17.0.0, which is the first version to contain the > fix. I think doing this unilaterally is kinda insane, LLVM 17 isn't even a year old. Debian testing doesn't have anything later than 16. Why does it need to be done unilaterally rather than just when the qspinlock stuff is built? > > Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1] > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ > Signed-off-by: Nathan Chancellor <nathan@kernel.org> If Nathan wrote the patch, you need to set him as the author of the patch :) > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > --- > > This patch was done by Nathan, I'm just sending it as an RFC to get quicker > feedbacks. > > I tested it successfully. > > Note that the build failure happens on the not-yet merged qspinlock > patchset. > > scripts/min-tool-version.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh > index 91c91201212c..e81eb7ed257d 100755 > --- a/scripts/min-tool-version.sh > +++ b/scripts/min-tool-version.sh > @@ -28,6 +28,8 @@ llvm) > echo 15.0.0 > elif [ "$SRCARCH" = loongarch ]; then > echo 18.0.0 > + elif [ "$SRCARCH" = riscv ]; then > + echo 17.0.0 > else > echo 13.0.1 > fi > -- > 2.39.2 > >
Hi Conor, On 17/07/2024 13:32, Conor Dooley wrote: > On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote: >> The following build failure happens when using LLVM < 17.0.0: >> >> kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets >> >> This is a known issue [1] so let's upgrade the minimal requirement for >> LLVM to the version 17.0.0, which is the first version to contain the >> fix. > I think doing this unilaterally is kinda insane, LLVM 17 isn't even a > year old. Debian testing doesn't have anything later than 16. Debian will very likely select the qspinlocks when available anyway, so they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 until some time right? So they'll probably update their infra to llvm >= 17 (and they'll probably do to take advantages of the new extensions). > Why does > it need to be done unilaterally rather than just when the qspinlock > stuff is built? We can do that indeed, it may happen again and we can keep requiring llvm 17 on a per-config basis. >> Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1] >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ >> Signed-off-by: Nathan Chancellor <nathan@kernel.org> > If Nathan wrote the patch, you need to set him as the author of the > patch :) I thought I did, how should I do that then? > >> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> >> --- >> >> This patch was done by Nathan, I'm just sending it as an RFC to get quicker >> feedbacks. >> >> I tested it successfully. >> >> Note that the build failure happens on the not-yet merged qspinlock >> patchset. >> >> scripts/min-tool-version.sh | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh >> index 91c91201212c..e81eb7ed257d 100755 >> --- a/scripts/min-tool-version.sh >> +++ b/scripts/min-tool-version.sh >> @@ -28,6 +28,8 @@ llvm) >> echo 15.0.0 >> elif [ "$SRCARCH" = loongarch ]; then >> echo 18.0.0 >> + elif [ "$SRCARCH" = riscv ]; then >> + echo 17.0.0 >> else >> echo 13.0.1 >> fi >> -- >> 2.39.2 >> >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote: > Hi Conor, > > On 17/07/2024 13:32, Conor Dooley wrote: > > On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote: > > > The following build failure happens when using LLVM < 17.0.0: > > > > > > kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets > > > > > > This is a known issue [1] so let's upgrade the minimal requirement for > > > LLVM to the version 17.0.0, which is the first version to contain the > > > fix. > > I think doing this unilaterally is kinda insane, LLVM 17 isn't even a > > year old. Debian testing doesn't have anything later than 16. > > > Debian will very likely select the qspinlocks when available anyway, so > they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 until some > time right? So they'll probably update their infra to llvm >= 17 (and > they'll probably do to take advantages of the new extensions). What I mean is that you are going to prevent people building the kernel with llvm on machines running anything but very recent rolling-release distros. Your patch would stop most developers, including those who don't care about your qspinlock stuff, even build testing with the version of LLVM that their distro provides. I'm not talking about distros building kernels in their build infrastructure. > > > > Why does > > it need to be done unilaterally rather than just when the qspinlock > > stuff is built? > > > We can do that indeed, it may happen again and we can keep requiring llvm 17 > on a per-config basis. > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1] > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ > > > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > > If Nathan wrote the patch, you need to set him as the author of the > > patch :) > > > I thought I did, how should I do that then? > > > > > > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> > > > --- > > > > > > This patch was done by Nathan, I'm just sending it as an RFC to get quicker > > > feedbacks. > > > > > > I tested it successfully. > > > > > > Note that the build failure happens on the not-yet merged qspinlock > > > patchset. > > > > > > scripts/min-tool-version.sh | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh > > > index 91c91201212c..e81eb7ed257d 100755 > > > --- a/scripts/min-tool-version.sh > > > +++ b/scripts/min-tool-version.sh > > > @@ -28,6 +28,8 @@ llvm) > > > echo 15.0.0 > > > elif [ "$SRCARCH" = loongarch ]; then > > > echo 18.0.0 > > > + elif [ "$SRCARCH" = riscv ]; then > > > + echo 17.0.0 > > > else > > > echo 13.0.1 > > > fi > > > -- > > > 2.39.2 > > > > > > > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote: > > > Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1] > > > Reported-by: kernel test robot <lkp@intel.com> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ > > > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > > If Nathan wrote the patch, you need to set him as the author of the > > patch :) > > > I thought I did, how should I do that then? $ shazam 20240717-synapse-decade-a0d41bd7afce@spud Grabbing thread from lore.kernel.org/all/20240717-synapse-decade-a0d41bd7afce@spud/t.mbox.gz Checking for newer revisions Grabbing search results from lore.kernel.org Analyzing 4 messages in the thread Looking for additional code-review trailers on lore.kernel.org Checking attestation on all messages, may take a moment... --- ✓ [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0 + Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- ✓ Signed: DKIM/rivosinc-com.20230601.gappssmtp.com (From: alexghiti@rivosinc.com) --- Total patches: 1 --- Applying: riscv: Allow to build only with LLVM >= 17.0.0 $ git show commit f64f8420ca518b5dde35224cfff7ccfd14e5b496 (HEAD) Author: Alexandre Ghiti <alexghiti@rivosinc.com> Date: Wed Jul 17 13:17:16 2024 +0200 riscv: Allow to build only with LLVM >= 17.0.0 The following build failure happens when using LLVM < 17.0.0: kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets This is a known issue [1] so let's upgrade the minimal requirement for LLVM to the version 17.0.0, which is the first version to contain the fix. Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1] Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> When you commited it, you needed to set --author. Just adding his signoff is not sufficient. Cheers, Conor.
On Wed, Jul 17, 2024 at 01:06:39PM +0100, Conor Dooley wrote: > On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote: > > Hi Conor, > > > > On 17/07/2024 13:32, Conor Dooley wrote: > > > On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote: > > > > The following build failure happens when using LLVM < 17.0.0: > > > > > > > > kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets > > > > > > > > This is a known issue [1] so let's upgrade the minimal requirement for > > > > LLVM to the version 17.0.0, which is the first version to contain the > > > > fix. > > > I think doing this unilaterally is kinda insane, LLVM 17 isn't even a > > > year old. Debian testing doesn't have anything later than 16. > > > > > > Debian will very likely select the qspinlocks when available anyway, so > > they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 until some > > time right? So they'll probably update their infra to llvm >= 17 (and > > they'll probably do to take advantages of the new extensions). > > What I mean is that you are going to prevent people building the kernel > with llvm on machines running anything but very recent rolling-release > distros. Your patch would stop most developers, including those who don't > care about your qspinlock stuff, even build testing with the version of > LLVM that their distro provides. I'm not talking about distros building > kernels in their build infrastructure. > > > > > > > > Why does > > > it need to be done unilaterally rather than just when the qspinlock > > > stuff is built? > > > > > > We can do that indeed, it may happen again and we can keep requiring llvm 17 > > on a per-config basis. Nathan pointed out to me that I misunderstood the build failure, and that it happens whether or not the option is enabled. /sigh.
On 17/07/2024 13:41, Alexandre Ghiti wrote: > Hi Conor, > > On 17/07/2024 13:32, Conor Dooley wrote: >> On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote: >>> The following build failure happens when using LLVM < 17.0.0: >>> >>> kernel/sched/core.c:11873:7: error: cannot jump from this asm goto >>> statement to one of its possible targets >>> >>> This is a known issue [1] so let's upgrade the minimal requirement for >>> LLVM to the version 17.0.0, which is the first version to contain the >>> fix. >> I think doing this unilaterally is kinda insane, LLVM 17 isn't even a >> year old. Debian testing doesn't have anything later than 16. > > > Debian will very likely select the qspinlocks when available anyway, > so they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 > until some time right? So they'll probably update their infra to llvm > >= 17 (and they'll probably do to take advantages of the new extensions). > > >> Why does >> it need to be done unilaterally rather than just when the qspinlock >> stuff is built? > > > We can do that indeed, it may happen again and we can keep requiring > llvm 17 on a per-config basis. > > >>> Link: >>> https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 >>> [1] >>> Reported-by: kernel test robot <lkp@intel.com> >>> Closes: >>> https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/ >>> Signed-off-by: Nathan Chancellor <nathan@kernel.org> >> If Nathan wrote the patch, you need to set him as the author of the >> patch :) > > > I thought I did, how should I do that then? > > >> >>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> >>> --- >>> >>> This patch was done by Nathan, I'm just sending it as an RFC to get >>> quicker >>> feedbacks. >>> >>> I tested it successfully. >>> >>> Note that the build failure happens on the not-yet merged qspinlock >>> patchset. >>> >>> scripts/min-tool-version.sh | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh >>> index 91c91201212c..e81eb7ed257d 100755 >>> --- a/scripts/min-tool-version.sh >>> +++ b/scripts/min-tool-version.sh >>> @@ -28,6 +28,8 @@ llvm) >>> echo 15.0.0 >>> elif [ "$SRCARCH" = loongarch ]; then >>> echo 18.0.0 >>> + elif [ "$SRCARCH" = riscv ]; then >>> + echo 17.0.0 >>> else >>> echo 13.0.1 >>> fi >>> -- >>> 2.39.2 >>> >>> >>> >>> _______________________________________________ >>> linux-riscv mailing list >>> linux-riscv@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-riscv > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv So we discussed that during the patchwork meeting and this patch is not wanted, the idea is rather to get rid of the build error. I was given a few ideas so I'll try those and we'll see what the resulting code looks like, hopefully not ugly otherwise I'll re-open the discussion :) Thanks Nathan for the patch, Alex
diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh index 91c91201212c..e81eb7ed257d 100755 --- a/scripts/min-tool-version.sh +++ b/scripts/min-tool-version.sh @@ -28,6 +28,8 @@ llvm) echo 15.0.0 elif [ "$SRCARCH" = loongarch ]; then echo 18.0.0 + elif [ "$SRCARCH" = riscv ]; then + echo 17.0.0 else echo 13.0.1 fi