kernel config BPI-R4 - compress kernel modules on disc #8538
Conversation
WalkthroughAdded three kernel configuration options to config/kernel/linux-filogic-current.config: CONFIG_MODULE_COMPRESS=y, CONFIG_MODULE_COMPRESS_ZSTD=y, and CONFIG_MODULE_DECOMPRESS=y. These lines were inserted near existing module-related options (e.g., CONFIG_MODULES, CONFIG_MODULE_UNLOAD). No existing lines were modified or removed; only additions were made. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
size/small
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
coderabbitai
bot
requested review from
chraac,
EvilOlaf,
igorpecovnik,
teknoid and
Tonymac32
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
config/kernel/linux-filogic-current.config (2)
111-113: Be explicit about mutually exclusive choices to avoid merge ambiguityOptional but helps readability during merges: explicitly mark the unused alternatives as not set next to these lines.
Apply this minimal diff:
CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_COMPRESS=y CONFIG_MODULE_COMPRESS_XZ=y CONFIG_MODULE_DECOMPRESS=y +# Explicitly prefer XZ over other algorithms for modules +# CONFIG_MODULE_COMPRESS_ZSTD is not set +# CONFIG_MODULE_COMPRESS_GZIP is not set
111-113: Consider Zstd if boot-time is a priority (trade-off note, not a blocker)XZ yields better on-disk compression; Zstd usually loads faster on low-power cores with a modest size penalty. If you later optimize for boot/module load latency over image size, switching to Zstd is a one-line change.
Alternative diff (for future consideration, not required for this PR):
-CONFIG_MODULE_COMPRESS_XZ=y +# CONFIG_MODULE_COMPRESS_XZ is not set +CONFIG_MODULE_COMPRESS_ZSTD=y
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
config/kernel/linux-filogic-current.config(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
🔇 Additional comments (3)
config/kernel/linux-filogic-current.config (3)
111-113: Change is coherent: XZ-compressed modules with in-kernel decompressionEnabling CONFIG_MODULE_COMPRESS together with CONFIG_MODULE_COMPRESS_XZ and CONFIG_MODULE_DECOMPRESS is a sensible trio. Given this config already has CONFIG_SQUASHFS_XZ=y, the XZ decompressor is already linked, so the incremental kernel bloat for module decompression should be minimal while improving robustness on systems where userspace kmod may not decompress. Looks good.
111-113: No module compression conflicts detected across config fragmentsI ran the suggested ripgrep checks across config/kernel and confirmed that no other fragment selects
CONFIG_MODULE_COMPRESS_ZSTD=yorCONFIG_MODULE_COMPRESS_GZIP=y. Therefore, theCONFIG_MODULE_COMPRESS_XZ=ysetting inlinux-filogic-current.configwon’t be overridden by another fragment.
111-113: No additional CI checks required—xz-utilsis already a host dependencyThe package
xz-utilsappears under compressors inadaptative_prepare_host_dependencieswithinlib/functions/host/prepare-host.sh(around line 210), guaranteeing thatxz(1)is installed in every build container. Per our “fail-fast” philosophy, declaring a host dependency is sufficient—no extra runtime check is needed.• File:
lib/functions/host/prepare-host.sh
• Line ~210:unzip zip pigz xz-utils pbzip2 lzop zstd
|
Thank you for picking this up. Very valuable stuff. I will not claim to be an expert on compression. My very quick and superficial research points to zstd possibly being the better choice here. advantage xz: more, higher compression, more mature I agree we should generalize this as wide as possible, no special-casing if at all possible. |
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
from
690c874 to
28b75d3
Compare
tabrisnet
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
config/kernel/linux-filogic-current.config (2)
111-111: Be explicit about mutually exclusive module compression options.To avoid surprises across kernel series (and merges), explicitly force other algorithms off when selecting compression to XZ.
Apply within this file:
CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y CONFIG_MODULE_COMPRESS=y CONFIG_MODULE_COMPRESS_XZ=y +CONFIG_MODULE_COMPRESS_ZSTD=n +CONFIG_MODULE_COMPRESS_GZIP=n CONFIG_MODULE_DECOMPRESS=y
113-113: MODULE_DECOMPRESS=y may be optional if kmod already handles XZ.Since kmod on your image supports XZ, in-kernel module decompression is a fallback rather than a requirement. It slightly grows the kernel image. If you need to keep the kernel tight (e.g., for small FIT or boot partitions), consider dropping this to rely on userspace. Given SQUASHFS_XZ=y is enabled, the XZ lib is likely already present, so the delta may be small—measure to decide.
Suggested quick measurement on two builds (with/without CONFIG_MODULE_DECOMPRESS):
- Compare vmlinux/vmlinuz sizes and boot time deltas.
- Confirm module loading still works early in initramfs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
config/kernel/linux-filogic-current.config(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rafayahmed317
PR: armbian/build#8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.
🧬 Code graph analysis (1)
config/kernel/linux-filogic-current.config (1)
lib/functions/compilation/armbian-kernel.sh (2)
armbian_kernel_config__disable_various_options(70-100)armbian_kernel_config_apply_opts_from_arrays(418-448)
🔇 Additional comments (2)
config/kernel/linux-filogic-current.config (2)
112-112: Consider Zstd for module compression if load latency is critical (optional)I wasn’t able to get any measurements from the provided script because
/lib/modules/$(uname -r)didn’t exist in the test environment. To properly compare XZ vs Zstd:
- Run the benchmark on the actual target device (where your modules are installed).
- Ensure the script’s kernel directory matches an existing version under
/lib/modules; you can list available versions with:ls /lib/modules- Once you’ve adjusted the path (or are on the correct device), re-run the timing loop to capture:
- Total
/lib/modules/$(uname -r)size- Per-module unload/load times and memory usage
If you find load/decompression speed matters more than raw on-disk size, swapping to Zstd may yield faster modprobe times and lower RAM peaks on resource-constrained SoCs. Capture before/after size and timing metrics to validate any change.
626-633: Action Required: Confirm generic XZ decoder is enabled in your final .configIt looks like you ran the grep against the repository root (no
.configpresent), rather than the generated kernel.configin your build output. The fragment inconfig/kernel/linux-filogic-current.configonly shows arch‐specific decoders being unset; to ensure module decompression with XZ works, you must verify that the generic decoder (CONFIG_XZ_DEC) is enabled in the actual build.config.• Locate the kernel’s generated
.config(usually at the top of your build directory).
• Run:grep -E '^CONFIG_XZ_DEC=y' /path/to/your/.config || echo "CONFIG_XZ_DEC is NOT set"• If that option is missing or set to
n, add or enable:+ CONFIG_XZ_DEC=yin your defconfig or via
make menuconfig.Once
CONFIG_XZ_DEC=yis confirmed, module decompression with XZ will work even though the arch‐specific decoders remain disabled.
any idea how to benchmark build performance? Mr rabbit mentioned that we have existing parts of the build machinery suppressing this. 😦 I'd have used GZIP but our |
I don't see any mention of the build process suppressing anything. What are you referring to? The logs actually have time stamps. What do you want to measure in terms of build performance? Speed? Then I assume "time ./compile.sh ..." should do the thing. You should do this a couple of times in a row, I guess to account for the effect of caches. |
Yeah, that's the obvious answer, but I expected the kernel compile & deb-packaging would get lost in the noise of the greater process. But trying to build just the kernel, manually, has been a fraught process [it defaulted to building as AMD64 for I appear to have linked the wrong comment from the bot
Clearly it works after as to why to benchmark? the benchmarking was just to understand the cost, as clearly I would need to make a best argument here. I'm also confused so far as to why my |
|
zstd produced a 30MB dpkg vs 28 for XZ... so it would be worthwhile |
Ryzen 5950X |
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
from
28b75d3 to
39cc0df
Compare
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
from
39cc0df to
7b14e80
Compare
regarding the nitpick comments: |
|
Tip For best results, initiate chat on the files or code changes. @tabrisnet Thank you for the clarification! You're absolutely right on both points: a) Since the kernel config file is autogenerated from manual inputs and gets regenerated by maintainers, adding comments would indeed be futile as they'd be lost in future updates. b) As an overlay configuration on top of the arch The automated review's nitpick suggestions weren't contextually appropriate for this type of configuration file. Your approach of simply enabling the desired compression method without explicit negation of alternatives is the correct pattern for Armbian's overlay configs. I noticed your excellent benchmarking work in the discussion - the ZSTD compression results showing ~4.6 seconds for 266 modules with 75.36% compression ratio, and your consideration of the trade-offs between XZ's better compression ratio versus ZSTD's faster decompression. The practical impact analysis (25 minutes across ~300 boards) provides valuable context for the build pipeline considerations. ✏️ Learnings added
🧠 Learnings used |
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
2 times, most recently
from
5d8611c to
651891e
Compare
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
2 times, most recently
from
fe4933e to
d3252fa
Compare
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
2 times, most recently
from
c480b66 to
00a1127
Compare
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
from
00a1127 to
f3d98d6
Compare
tabrisnet
force-pushed
the
tabrisnet_bpir4_koxz
branch
from
f3d98d6 to
8df6aed
Compare
|
✅ This PR has been reviewed and approved — all set for merge! |
Inspiration for #8560, where much more testing was done also.
Use
ZSTD[originallyXZ] asmodprobe --versionsaysReason to do this:
So it saves ~20MiB on disc
and in the.linux-imagepackage as wellReasons to not do this:
a) paranoia about the
xzlibrary compromise [not really an XZ thing, just potential bad association].a.1) Is this a new kernel dependency for us? It should already be a userspace dependency.
On my NanoPC-T6,
zgrep /proc/config.gzmentionsDECOMPRESS_XZ, so I don't think this is new.b) possibility of tooling failures [
MODULE_DECOMPRESSmay save us, maybe it won't] on older versions of Armbian/debian.b.1)
MODULE_DECOMPRESSlikely makes the kernel base-image a hair bigger.c) could use
zstdinstead.zstdis already included forzram.This is the kind of thing to consider making standard across all board configs. But I don't own all of the boards so can't hardly test them all. OTOH, it shouldn't be interesting per-board/chip, only per kernel & userspace. We could just ignore the vendor kernels.
Description
see above for justification/intent.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
/proc/moduleswasn't empty.Checklist:
Please delete options that are not relevant.