Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux-cachyos: Default to ThinLTO #304

Merged
merged 4 commits into from
Oct 5, 2024
Merged

linux-cachyos: Default to ThinLTO #304

merged 4 commits into from
Oct 5, 2024

Conversation

1Naim
Copy link
Member

@1Naim 1Naim commented Oct 1, 2024

Second and final part of #286. The default kernel will now use clang + ThinLTO as its default configuration.

To accomodate this change, I have added a conditional statement for each _package function to replace existing LTO kernels that are already installed to smoothen the transition. Furthermore, I have modified the build script to ensure that all PKGBUILDs will build with GCC first before building the LTO kernels.

We should do a staged rollout for this change to cachyos-testing-v3 -> cachyos-znver4 -> all repos to ensure that everything works correctly (most notably dkms modules, even though they compile successfully, we should ensure complete functionality)

Closes #286

@1Naim 1Naim requested review from ptr1337 and ventureoo October 1, 2024 00:59
@1Naim 1Naim marked this pull request as draft October 1, 2024 01:25
@1Naim 1Naim force-pushed the default-thinlto branch 2 times, most recently from 6615efd to 7e23248 Compare October 1, 2024 02:02
@1Naim 1Naim marked this pull request as ready for review October 1, 2024 02:03
@1Naim
Copy link
Member Author

1Naim commented Oct 1, 2024

Okay so --printsrcinfo doesn't seem to honor the if statement inside _package but the built package does.

-pkgname = linux-cachyos
+pkgname = linux-cachyos-gcc
        pkgdesc = The Linux SCHED-EXT + BORE + Cachy Sauce Kernel by CachyOS with other patches and improvements kernel and modules
        depends = coreutils
        depends = kmod
@@ -52,9 +48,9 @@ pkgname = linux-cachyos
        provides = linux-cachyos-lto=6.11.1-2
        replaces = linux-cachyos-lto
❯ pacman -Qi linux-cachyos-gcc
Installed From  : None
Name            : linux-cachyos-gcc
Version         : 6.11.1-2
...
Provides        : VIRTUALBOX-GUEST-MODULES  WIREGUARD-MODULE  KSMBD-MODULE  UKSMD-BUILTIN
...
Required By     : linux-cachyos-gcc-headers
Optional For    : None
Conflicts With  : None
Replaces        : None
❯ pacman -Qi linux-cachyos
Installed From  : None
Name            : linux-cachyos
Version         : 6.11.1-2
...
Provides        : VIRTUALBOX-GUEST-MODULES  WIREGUARD-MODULE  KSMBD-MODULE  UKSMD-BUILTIN  linux-cachyos-lto=6.11.1-2
...
Replaces        : linux-cachyos-lto

linux-cachyos/PKGBUILD Outdated Show resolved Hide resolved
@ventureoo
Copy link
Member

Is this supposed to be for use on all kernels or just linux-cachyos? I don't see any changes in other PKGBUILDs.

@1Naim
Copy link
Member Author

1Naim commented Oct 1, 2024

Is this supposed to be for use on all kernels or just linux-cachyos? I don't see any changes in other PKGBUILDs.

@ptr1337 said only for the default kernel.

edit: I'll update the PR accordingly if the decision has changed to be for all kernels.

linux-cachyos/PKGBUILD Outdated Show resolved Hide resolved
Copy link
Member

@ventureoo ventureoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@1Naim
Copy link
Member Author

1Naim commented Oct 1, 2024

Another thing to consider is _use_lto_suffix. Since we will use LTO in the default kernel, it will not have a -lto suffix, instead the gcc build kernel will have a -gcc suffix instead. Considering this thought, should we remove it altogether, see below:

diff --git a/linux-cachyos/PKGBUILD b/linux-cachyos/PKGBUILD
index 0be37b25..8d02a9c8 100644
--- a/linux-cachyos/PKGBUILD
+++ b/linux-cachyos/PKGBUILD
@@ -111,13 +111,6 @@ _use_auto_optimization=${_use_auto_optimization-y}
 # "none: disable LTO
 _use_llvm_lto=${_use_llvm_lto-thin}
 
-# Use suffix -lto only when requested by the user
-# Enabled by default.
-# y - enable -lto suffix
-# n - disable -lto suffix
-# https://github.com/CachyOS/linux-cachyos/issues/36
-_use_lto_suffix=${_use_lto_suffix-y}
-
 # KCFI is a proposed forward-edge control-flow integrity scheme for
 # Clang, which is more suitable for kernel use than the existing CFI
 # scheme used by CONFIG_CFI_CLANG. kCFI doesn't require LTO, doesn't
@@ -147,12 +140,12 @@ _is_lto_kernel() {
 # Build a debug package with non-stripped vmlinux
 _build_debug=${_build_debug-}
 
-if [[ "$_use_llvm_lto" = "thin" || "$_use_llvm_lto" = "full" ]] && [ "$_use_lto_suffix" = "y"  ]; then
-    _pkgsuffix=cachyos
+if [ -n "$_use_llvm_lto" ]; then
+    _pkgsuffix=cachyos-gcc
     pkgbase="linux-$_pkgsuffix"
 
-elif [ -n "$_use_llvm_lto" ]  ||  [[ "$_use_lto_suffix" = "n" ]]; then
-    _pkgsuffix=cachyos-gcc
+else
+    _pkgsuffix=cachyos
     pkgbase="linux-$_pkgsuffix"
 fi
 _major=6.11

@1Naim
Copy link
Member Author

1Naim commented Oct 1, 2024

Another alternative is to modify it to use_gcc_suffix instead. I will make changes with the optimal solution.

@1Naim 1Naim requested a review from ventureoo October 1, 2024 12:29
@1Naim
Copy link
Member Author

1Naim commented Oct 1, 2024

I have settled on keeping _use_lto_suffix and adding an additional _use_gcc_suffix. Refer to commit message for technical details.

@ptr1337 ptr1337 self-assigned this Oct 1, 2024
@ptr1337
Copy link
Member

ptr1337 commented Oct 4, 2024

I will push this for now to the Zen4 repository

1Naim added 3 commits October 6, 2024 02:16
Signed-off-by: Eric Naim <dnaim@cachyos.org>
As pointed out by @ventureoo, this condition is used a lot and it would be better to move it to its own function
so if a modification is needed to the condition, it only needs to be done once.

Signed-off-by: Eric Naim <dnaim@cachyos.org>
Use `$_use_gcc_suffix` instead of `$_use_lto_suffix` since LTO kernel is going to be the default now.
The latter variable will be preserved for users that still want to use it.

I have modified the if statement with the logic:
1. If using LTO + enabled LTO suffix -> cachyos-lto
2. If no LTO + no kCFI + enabled GCC suffix -> cachyos-gcc
3. All other combinations -> cachyos

The main rationale behind (2) is that kCFI isn't LTO but it also isn't gcc.

I have also moved $pkgbase outside of the if else condition because it is the same regardless of condition

Signed-off-by: Eric Naim <dnaim@cachyos.org>
…ompiler for the default kernel

Signed-off-by: Eric Naim <dnaim@cachyos.org>
Copy link
Member

@ptr1337 ptr1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Naim!

@ptr1337 ptr1337 merged commit a9ade2e into master Oct 5, 2024
2 checks passed
@ptr1337 ptr1337 deleted the default-thinlto branch October 5, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Using Clang (ThinLTO) for the default kernel in the cachyos repository
3 participants