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

DietPi-PreBoot | Separate first boot and regular boot steps #2833

Merged
merged 21 commits into from
Jun 4, 2019
Merged

DietPi-PreBoot | Separate first boot and regular boot steps #2833

merged 21 commits into from
Jun 4, 2019

Conversation

FredericGuilbault
Copy link
Contributor

#2791

This PR separate the pre-network part of the first boot process into it's own .service file.

( post-login part is yet to be done. )

Tested on RPI , Qemu-ARM and Qemu-i686

@Fourdee
Copy link
Collaborator

Fourdee commented May 20, 2019

@FredericGuilbault

Very nice 👍

Many thanks for the PR, i'll take a look and test.

dietpi/dietpi-firstboot Outdated Show resolved Hide resolved
dietpi/dietpi-firstboot Outdated Show resolved Hide resolved
Copy link
Collaborator

@Fourdee Fourdee left a comment

Choose a reason for hiding this comment

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

Nice work 👍

Some minor changes required from my end, mostly for consistency.

@FredericGuilbault
Copy link
Contributor Author

Im glad to have your feedback. Im on a contract right now but Ill look at it in the next weeks

@MichaIng MichaIng self-requested a review May 21, 2019 22:01
@MichaIng MichaIng added this to the v6.25 milestone May 21, 2019
dietpi/boot Show resolved Hide resolved
dietpi/preboot Outdated Show resolved Hide resolved
@MichaIng
Copy link
Owner

I just measured the execution time, to see if there is any effect of unused code:

root@VM-Stretch:/tmp# time for i in {1..10}; do ./preboot; done
...
real    0m11.830s
user    0m0.868s
sys     0m0.312s
root@VM-Stretch:/tmp# time for i in {1..10}; do /DietPi/dietpi/preboot; done
...
real    0m11.784s
user    0m0.924s
sys     0m0.212s
  • So inerte code has no influence. I removed the whole install stage == -1 part + RPi_CPU_Clocks as well for this, as for completeness this should be done as well. As you can see no performance benefit (./preboot the one from this PR with above further reduction).
  • However to have the boot script code cleaner and save some RAMdisk space should be reason enough.

Largest impact is of course the two child scripts (CPU governor + LED settings) that are executed in subshell and load dietpi-globals in case. Actually it should be at least possible to skip led settings, an additional setting to revert to system defaults, which removes the settings file and then scripts the execution. Actually we could move this as well as the CPU governor adjustment into an own systemd unit each. It does not really matter when they run, just after DietPi-RAMdisk. So they can be Type=simple to not delay any other boot scripts. This also makes it again simpler to revert to system defaults => disable the related systemd unit. Once applied settings (settings file) can be preserved then while toggling the service.

Finally with this the preboot script can be removed completely. Only thing that we need to run somewhere is:

  • /DietPi/dietpi/func/dietpi-obtain_hw_model
  • (( $G_HW_MODEL < 10 )) && command -v amixer &> /dev/null && amixer set PCM -- -010

Both are only required for RPi, hw_model estimation additionally is a failsafe thing for other devices as well.

  • hw_model, if it runs at boot for all devices, it makes sense to run it as early as possible. Not beautiful but possible would be to merge it into dietpi-ramdisk 🤔. At least there it runs at the earliest possible point.
  • amixer, I do not really understand what this is for @Fourdee ? Why volume needs adjustment and why this cannot be done e.g. via asound.conf or such?
    However it should be possible to move this from preboot to boot script.

@Fourdee
Copy link
Collaborator

Fourdee commented May 22, 2019

@MichaIng

amixer, I do not really understand what this is for @Fourdee ? Why volume needs adjustment and why this cannot be done e.g. via asound.conf or such?
However it should be possible to move this from preboot to boot script.

This is resolved with current kernel under stretch. Can be removed.
It was previously needed as the PCM volume would reset (AFAIK, was a year+ ago lol) to lower volume after reboot. Not any more 👍

Done 🈯 #2833 (comment)

Copy link
Contributor Author

@FredericGuilbault FredericGuilbault left a comment

Choose a reason for hiding this comment

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

In the commit 7760a2d The firstboot script have move to /var/lib/dietpi/services/

I think this break @Fourdee commit c0aa1e8

FredericGuilbault and others added 2 commits May 22, 2019 16:54
+ DietPi-Update | Revert firstboot script removal. This has now been moved to: /var/lib/dietpi/services/
@MichaIng
Copy link
Owner

@FredericGuilbault
Is it okay if we do commits to your branch? Usually I don't like it but it is possible, when a PR to our branch is opened. Thus I'm asking 😉.

@FredericGuilbault
Copy link
Contributor Author

Sure, no problem. I think you could also create a branch and modify this pullrequest to change it to the new branch and merge it. But I don't mind it's up to you.

MichaIng added 6 commits June 1, 2019 18:34
+ DietPi-PREP | Spelling
+ DietPi-Boot | Re-add swapfile creation on pre-installed images
+ DietPi-Boot | Failsafe network interface index estimation
+ DietPi-FirstBoot | Make dietpi-preboot.service a hard dependency, install to multi-user.target like all other boot scripts as well
+ DietPi-FirstBoot | Renamed to ".bash" file ending to make requirement clear, log to /tmp and leave moving to disk to DietPi-PostBoot
+ DietPi-FirstBoot | Add all "(( $G_DIETPI_INSTALL_STAGE == -1 ))" steps here
+ DietPi-FirstBoot | Rename to ".bash" file ending to make shell requirement clear
+ DietPi-FirstBoot | Do obtain network details here, which is already done in DietPi-PreBoot
+ DietPi-FirstBoot | Failsafe dietpi.txt and .network value grabbing, to avoid errors and instead apply defaults
+ DietPi-FirstBoot | Minor coding and alignment
+ DietPi-PreBoot | All firstrun setup steps have been moved to DietPi-FirstBoot
+ DietPi-PREP | Minor
@MichaIng MichaIng self-requested a review June 1, 2019 18:14
@MichaIng MichaIng requested a review from Fourdee June 1, 2019 18:31
+ DietPi-Boot | In attempt to resolve conflict due to: 64bd543#diff-620191171f47b4763a532e984acd2e8b
@MichaIng
Copy link
Owner

MichaIng commented Jun 1, 2019

@FredericGuilbault @Fourdee
Hmm, I am not able to resolve the conflict. The web editor is simply loading forever. It is only due to this tiny change: 64bd543#diff-620191171f47b4763a532e984acd2e8b

Will try it again later, but if you can do it, go for it, simply remove the "dev" part and fully use the version of this PR/branch, since I already committed the changes by Fourdee here as well: c28853c

EDIT: Made it, "HTTPS Everywhere" broke the web editor strangely 😄.

@MichaIng MichaIng changed the title Improvement/seperate first boot DietPi-PreBoot | Separate first boot and regular boot steps Jun 1, 2019
MichaIng added 2 commits June 4, 2019 02:28
+ DietPi-PREP | DietPi-FirstBoot needs to be enabled on all systems
+ DietPi-FirstBoot | Tiny
MichaIng added 2 commits June 4, 2019 02:53
+ CHANGELOG | DietPi-FirstBoot: First boot setup steps have been moved into a separate script and systemd unit
+ DietPi-FirstBoot | Fix RPi3 A+ default clocks
@MichaIng
Copy link
Owner

MichaIng commented Jun 4, 2019

Okay I merge the PR and run some tests.

@FredericGuilbault
Copy link
Contributor Author

Nice work 💃

@Fourdee
Copy link
Collaborator

Fourdee commented Jun 25, 2019

@FredericGuilbault

Please email me an address to [email protected]. I'll ship some DietPi stickers out to you for your excellent work on this PR.

Apologies for the delay ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants