Page MenuHomeFreeBSD

/etc/rc: optionally turn on new 'rcorder -p' option to run rc scripts in parallel.
Needs RevisionPublic

Authored by unitrunker_gmail.com on Sep 20 2020, 12:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Dec 20 2023, 7:01 AM
Unknown Object (File)
Dec 10 2023, 10:55 PM
Unknown Object (File)
Nov 14 2023, 12:15 PM
Unknown Object (File)
Nov 7 2023, 4:09 AM
Unknown Object (File)
Nov 4 2023, 6:23 AM
Unknown Object (File)
Oct 13 2023, 11:18 AM
Unknown Object (File)
Oct 6 2023, 3:04 AM

Details

Reviewers
ae
melifaro
avg
julian
0mp
pjd
cy
Group Reviewers
rc
Summary

Patch to /etc/rc to optionally turn on the new 'rcorder -p' option to run rc scripts in parallel.

See: https://reviews.freebsd.org/D25389

/etc/rc currently does not support output from 'rcorder -p'.

Make parallel startup optional by an entry in /etc/rc.conf:

rc_parallel_start="YES"

This name mirrors the current, existing variable "jail_parallel_start" that allows jails to start up in parallel. See rc.conf(5).

Test Plan

"AC" - Acceptance Criteria

AC 1:

rc_parallel_start="YES" causes /etc/rc to pass '-p' option to rcorder in the early and late passes. An rc_parallel_start that is undefined, "NO" or any value other than "YES", "TRUE", "ON", or "1" (using checkyesno) will cause /etc/rc to omit the '-p' option to rcorder.

Tested by booting with the "new" rcorder installed in /sbin/ and booting - with and without 'rc_parallel_start=YES' in /etc/rc.conf.
Tested by enabling 'info' messages (sysrc rc_info=YES) and adding 'info ${_rc_parallel}' to the /etc/rc script.

AC 2:

If rcorder returns multiple script paths per line separated by a space, /etc/rc will execute each script - with each script path passed to run_rc_script - as a background task. This behavior is independent of the rc_parallel_start option.

Tested by booting with the "new" rcorder installed in /sbin/ and booting - with and without 'rc_parallel_start=YES' in /etc/rc.conf.
Tested by enabling 'info' messages (sysrc rc_info=YES) and adding 'info ${_rc_group}' and 'info ${_rc_elem}' to the /etc/rc script.

AC 3:

After processing each line of output from rcorder, /etc/rc will pause by calling 'wait' to allow all rc script tasks to complete before proceeding to the next line of output from rcorder. This behavior is independent of the rc_parallel_start option.

Tested by booting with the "new" rcorder installed in /sbin/ and booting - with and without 'rc_parallel_start=YES' in /etc/rc.conf.
Tested by enabling 'info' messages (sysrc rc_info=YES) and adding 'info ${_rc_group}' and 'info ${_rc_elem}' to the /etc/rc script.

AC 4:

In the presence of the "old" rcorder (-p not supported), this /etc/rc script still functions correctly but without parallel execution.

Tested by booting with the "old" rcorder installed in /sbin/ and booting - with and without 'rc_parallel_start=YES' in /etc/rc.conf.

All 'info' statements have been removed from the patch.

Note: in the future, parallel start-up may become the default. This does not eliminate the need for the rc_parallel_start variable. Supporting rc_parallel_start="NO" in /etc/rc.conf remains useful for debugging rc scripts.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

pjd requested changes to this revision.Jun 20 2023, 10:53 PM
pjd added a subscriber: pjd.
pjd added inline comments.
head/libexec/rc/rc
135

Can you move ${_rc_parallel} after ${skip_firstboot}?

This revision now requires changes to proceed.Jun 20 2023, 10:53 PM
cy requested changes to this revision.Jun 21 2023, 4:37 AM
cy added a subscriber: cy.
cy added inline comments.
head/libexec/rc/rc
104

You will need to save IFS prior to this line, i.e. OFS="$IFS".

149

You will need to restore IFS here, i.e. IFS="$OFS".

Revisiting. I need a solid way to test this.

One idea is to create a dozen or so fake rc scripts and isolate startup to only see those scripts. We aren't booting the host, but instead testing the rc stack. Each fake script records its complete environment to a log file. The point of the test is to compare environments when rc_parallel_start="YES" vs. rc_parallel_start="NO". A script gets a "pass" if the environments match. Hoping I can do this inside a jail.

Does this sound reasonable?

I have something in a git branch that does this. I'll need to look at it again to see how different this is from my code.

Thank you @cy. The other thing I'd like to do is ...

instead of ...

OFS="$IFS"
<do some work>
IFS="$OFS"

and worry about someone else using "OFS" is to do something like this:

(IFS=whatever; <do some work>)

This might require refactoring the for-loop into a separate function but I think would be cleaner overall.

Another thing ... lines 108-109

		run_rc_script ${_rc_elem} ${_boot} &
		_rc_elem_done="${_rc_elem_done}${_rc_elem} "

I'm thinking the _rc_elem_done variable should not be appended until after the run_rc_script statement is complete.

This question terrifies me to even ask.

Are there rc scripts that make use of "breadcrumbs"? By breadcrumb, I mean an earlier script sets a variable that a later script uses.