-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Pass all modules from O2/O3 (optimized for speed) to Os (optimize for space) #1121
Conversation
da3e653
to
86d120e
Compare
Rebased on master to reuse CircleCI 4.11 fixed cache by #1122 |
86d120e
to
c0fab00
Compare
Checkspots:
Todo:
|
Not all modules are passed from default O2 to Os as of original commit:
|
Master at 2cfa940 Comparison is done on downloaded initrd.cpio.xz from CircleCI artifact, doing:
Current optimization of this PR effect:
|
Adding gpg2 and cryptsetup2 |
master:
Actual PR:
4786840 - 4361880 = 424960 |
b0890a5
to
9e54fc4
Compare
Impact on changes in build logs: master vs this pr:
|
@JonathonHall-Purism @SergiiDmytruk : LGTM, not perfect, but interesting 400kb gain for all boards as observed under #1121 (comment) This would permit us to start adding other stuff, including (while needing prioritizing)
Recommendations on this PR?
Where some previous |
The changes look sane to me. |
@tlaurion Agree the ~400 KB is significant, however GPG key generation during OEM reset now takes much longer - on Mini v2, went from ~4 minutes to ~7 minutes. I agree -Os makes sense as a default otherwise. The only other thing I can think of that might be noticeably affected is LUKS re-encryption - I have not actually tried this at all yet, not sure if it is typically I/O or CPU bound.
|
@JonathonHall-Purism : Houla. Will test re-ownership and check re-encryption and gpg in-smartcard generation, not sure how this could be related to this change though, since keys are generated inside of the USB Security dongle; I would understand if in-heads generated keys were affected though, and cryptsetup-reencrypt speed being maybe affected. Re-encryption is both CPU and IO bound, since blocks are being rewritten in direct-io mode from cryptsetup-reencrypt. Will check results on same disk and same USB Security dongle with master and this PR and report back results editing this post. EDIT:
This means a variation of
|
Thanks @tlaurion , I ran several more tests to get some better data. Bottom line is that the variations in GPG key reset seem to be normal and it was just coincidence in the first few tests that the -Os tests happened to be longer. With that in mind, and LUKS re-encrypt confirmed fine, this PR looks good to me. (The failed LUKS tests were apparently just due to lack of space for key slot expansion or something on that disk, unrelated to this PR, I took the 970 EVO from the L14 and put it in the mini to continue.) Thanks for confirming this with me! |
@SergiiDmytruk @JonathonHall-Purism One area of concern is still:
For busybox, my Make fu is not good enough to understand why some are passing -02 and some ware passing -0s. I will clean the PR to remove modules which have no change with this PR to clear things up and force push again for one last review before merging. This freed space is important. |
Also, just to be clear, hardcoding CFLAGS="-Os" is removing debugging information (no more |
Current state of master:
Current state of this PR per 9e54fc4:
Modules explicitely passing Os in the builds:
Modules still having O2/O3 instead of Os in the build:
|
Adding
Results in increased gain:
|
Rebased on current master for comparison with additional freed space for maximized boards |
Will compare t420-htop-maximized from master to this pr (this board is the one having less space avail):
So 622744-1077848=-455104 |
…, yubikey test regression for oem-factory-reset, optimized for space (03-O2->Os) and fix for sh: argument expected, with local CONFIG_DEBUG_OUTPUT enabled and fused in ROM. Includes linuxboot#1317, linuxboot#1121, linuxboot#1312, linuxboot#1305 for test on daily driver
…ix-sh_argument_expected-yubikey-oem-factory-reset_dasharo-flashrom Daily driver test fo x230-hotp-maximized on coreboot 4.19, with debug, yubikey test regression for oem-factory-reset, optimized for space (03-O2->Os) and fix for sh: argument expected, with local CONFIG_DEBUG_OUTPUT enabled and fused in ROM. Includes linuxboot#1317, linuxboot#1121, linuxboot#1312, linuxboot#1305, linuxboot#1251 for test on daily driver
…ix-sh_argument_expected-yubikey-oem-factory-reset_dasharo-flashrom Daily driver test fo x230-hotp-maximized on coreboot 4.19, with debug, yubikey test regression for oem-factory-reset, optimized for space (03-O2->Os) and fix for sh: argument expected, with local CONFIG_DEBUG_OUTPUT enabled and fused in ROM. Includes osresearch#1317, osresearch#1121, osresearch#1312, osresearch#1305, osresearch#1251 for test on daily driver
Using x230-hotp-maximized from CircleCI builds as of now. Nothing to report as of now as regression. |
…gument_expected-yubikey-oem-factory-reset_dasharo-flashrom - Additional -O2 -O3 -> -Os fixes to linuxboot#1121
|
master:
pr:
4743896 - 5198488 = -454592 (gain of 0.45Mb) diffoscope output for tools.cpio differences (first diff) for x230-hotp-maximized (master vs pr) We can see that dasharo/flashrom is bigger then master's:
|
master:
pr:
68184 - 37340 = loss of 30844 (flashrom/flashrom to dasharo/flashrom really increased :() diffoscope (to see whiptail and dependencies (newt) diffs in size)
|
Seems like we can go to flashrom/dasharo but space is really really tight even with this merge. |
16904f3
to
9f2a8a0
Compare
Removed dasharo/flashrom. Ok so we can compare https://app.circleci.com/pipelines/github/osresearch/heads/526/workflows/bdef2e57-923b-441e-94ff-ad10707bd3a5 with https://app.circleci.com/pipelines/github/tlaurion/heads/1523/workflows/92558d5a-87e0-4e8f-8229-fedfcc4c6715 Taking x230-hotp-maximized tools.cpio for both.
Extract of diffoscope diffing cpio file listing only (first report):
Extract from top to end of filelist of Mardown output from last command:
|
So on modules, last validation for success:
|
For next forced push commit, remove non-working attempted changes for the following modules/*:
and compare t430-legacy boards (
|
Adresses @easrentai suggestion to pass modules build optimization for space here: linuxboot#590 (comment) - Uniformized module's $(CROSS_TOOLS) being passed as environment variable, prior of ./configure call Doesn't work for: - busybox (HOSTCXXFLAGS="-Os" attempted prior of ./configure call) - zlib (CFLAGS="-Os" attempted prior of ./configure call) - npth (CFLAGS="-Os" attempted prior of ./configure call)
but this time for t430-hotp-legacy, which is whiptail based for now:
Comparison of diffoscope extract, only for newt:
Success for tackled modules. |
Leaving changes for gpg and cryptsetup1 even if not tested. Can revert if desired. |
Adresses @aesrentai suggestion to pass modules optimization from O2 (performance) to Os (space) here: #590 (comment)
Will comment on gains after build succeeds.
Still impossible as of now to build xx30-flash boards on top of linux 5.10.4......EDIT: without kernel networking support, this now passes.