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

initramfs bootchart #3098

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

initramfs bootchart #3098

wants to merge 10 commits into from

Conversation

jakogut
Copy link
Contributor

@jakogut jakogut commented Apr 6, 2023


Contributor checklist

Reviewer Guidelines

  • When submitting a review, please pick:
    • 'Approve' if this change would be acceptable in the codebase (even if there are minor or cosmetic tweaks that could be improved).
    • 'Request Changes' if this change would not be acceptable in our codebase (e.g. bugs, changes that will make development harder in future, security/performance issues, etc).
    • 'Comment' if you don't feel you have enough information to decide either way (e.g. if you have major questions, or you don't understand the context of the change sufficiently to fully review yourself, but want to make a comment)

@jakogut jakogut force-pushed the bootchart-initramfs branch from e381564 to c516bc2 Compare April 6, 2023 01:38
@jakogut jakogut requested review from alexgg and acostach April 6, 2023 01:39
@jakogut jakogut marked this pull request as ready for review April 6, 2023 01:39
@jakogut jakogut force-pushed the bootchart-initramfs branch from c516bc2 to 7cfbc27 Compare April 6, 2023 01:39
@flowzone-app flowzone-app bot enabled auto-merge April 6, 2023 01:40
@jakogut jakogut force-pushed the bootchart-initramfs branch 3 times, most recently from a641112 to 7a93fb4 Compare April 6, 2023 17:41
@jakogut jakogut force-pushed the bootchart-initramfs branch 3 times, most recently from d7d46a5 to 6203584 Compare April 19, 2023 15:29
@jakogut jakogut force-pushed the bootchart-initramfs branch from 6203584 to b8bee59 Compare May 17, 2023 20:12
@jakogut jakogut force-pushed the bootchart-initramfs branch from b8bee59 to 11a006b Compare June 22, 2023 21:03
@jakogut jakogut force-pushed the bootchart-initramfs branch 2 times, most recently from 0456d08 to 8701ae1 Compare July 6, 2023 16:26
Copy link
Contributor

@alexgg alexgg left a comment

Choose a reason for hiding this comment

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

Let's try not to modify production code to introduce an OS development helper tool.

mount --move /sys $ROOTFS_DIR/sys

# remount proc in $ROOTFS_DIR instead of moving if systemd-bootchart is
# running, as it reads from /proc/schedstat
if pgrep systemd-bootchart; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Guessing pgrep is also included in the initramfs or it's an enabled busybox applet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexgg pgrep is provided by busybox, is there a way to add this as an RDEPENDS?

@@ -14,10 +14,10 @@ prepare_run() {
if ! mount -t tmpfs -o "nosuid,nodev,strictatime,mode=0755" tmpfs /run; then
warn "Couldn't mount /run so initramfs logs will not be persistent."
fi
mkdir -m 0755 /run/initramfs
mkdir -m 0755 /run/log
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any special reason for the name change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexgg By default, bootchart outputs to /run/log, so this directory needs created anyway. Rather than create both /run/initramfs and /run/log, I chose to consolidate on /run/log, which is more conventional.

@@ -17,7 +18,8 @@ SRC_URI:append = " \
"

do_install:append() {
install -m 0755 ${WORKDIR}/prepare ${D}/init.d/70-prepare
install -m 0755 ${WORKDIR}/prepare ${D}/init.d/00-prepare
Copy link
Contributor

Choose a reason for hiding this comment

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

Before we had:

000-console_null
00-debug
01-udev
70-prepare

I'd rather leave the prepare module after udev as it was before, but I am guessing prepare mounts somewhere where bootchar logs and we want bootchart to also include udev data, right?

I would prefer if the introduction of this bootchart module which is really only useful for OS development would not modify production code, like moving prepare before udev.

Copy link
Contributor Author

@jakogut jakogut Jul 17, 2023

Choose a reason for hiding this comment

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

@alexgg Correct, bootchart is intended to run as early as possible to profile the boot process, and udev (as well as many of the subprocesses it spawns) is one of the primary resource consumers during boot.

The prepare hook mounts a tmpfs at /run, which is where the bootchart output is written.

We could duplicate this in the bootchart hook in the run() function before running bootchart, but I found it cleaner to just mount /run earlier.

This should also result in more complete logs, especially when debugging udev rules, as we capture any output much earlier.

@jakogut jakogut requested a review from alexgg July 19, 2023 16:33
@jakogut jakogut force-pushed the bootchart-initramfs branch 2 times, most recently from 0ef57cb to 6117ea9 Compare August 2, 2023 21:32
@jakogut
Copy link
Contributor Author

jakogut commented Aug 2, 2023

@resin-jenkins retest this please

1 similar comment
@jakogut
Copy link
Contributor Author

jakogut commented Aug 2, 2023

@resin-jenkins retest this please

@jakogut jakogut force-pushed the bootchart-initramfs branch from 6117ea9 to bd7cf5d Compare August 10, 2023 23:50
@jakogut jakogut force-pushed the bootchart-initramfs branch from bd7cf5d to dfd3453 Compare August 25, 2023 17:56
@jakogut jakogut force-pushed the bootchart-initramfs branch from dfd3453 to 6760364 Compare December 5, 2024 18:47
@jakogut jakogut temporarily deployed to balena-staging.com December 5, 2024 18:48 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 5, 2024 18:48 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 5, 2024 18:48 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 5, 2024 18:48 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 5, 2024 18:48 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-staging.com December 10, 2024 00:29 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:05 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:05 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:05 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 01:19 — with GitHub Actions Inactive
@jakogut jakogut temporarily deployed to balena-cloud.com December 10, 2024 16:38 — with GitHub Actions Inactive
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.

2 participants