Bug 249192 - /etc/rc changes to support new rcorder -p option for parallel startup.
Summary: /etc/rc changes to support new rcorder -p option for parallel startup.
Status: Closed Not Accepted
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Baptiste Daroussin
URL: https://reviews.freebsd.org/D25389
Keywords: patch
Depends on:
Blocks:
 
Reported: 2020-09-08 11:41 UTC by unitrunker
Modified: 2023-12-16 05:16 UTC (History)
7 users (show)

See Also:


Attachments
modified /etc/rc script (not a proper patch file) (2.76 KB, application/octet-stream)
2020-09-09 10:36 UTC, unitrunker
no flags Details
/etc/rc supports rcorder -p for parallel startup. (2.09 KB, patch)
2020-09-10 05:23 UTC, unitrunker
no flags Details | Diff
/etc/rc supports rcorder -p for parallel startup. (2.09 KB, patch)
2020-09-15 18:20 UTC, unitrunker
no flags Details | Diff
$' ' is incorrect, it must include all whitespace (709 bytes, patch)
2021-02-25 18:53 UTC, Cy Schubert
no flags Details | Diff
parallel startup /etc/rc (2.76 KB, patch)
2023-12-16 05:16 UTC, Ken DEGUCHI
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description unitrunker 2020-09-08 11:41:44 UTC
This will be a patch to /etc/rc to optionally turn on the new '-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).

Acceptance criteria 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" (using checkyesno) will cause /etc/rc to omit the '-p' option to rcorder.

Acceptance criteria 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_sc_script - as a background task. This behavior is independent of the rc_parallel_start option.

Acceptance criteria 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.

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 will remain useful for debugging rc scripts.

Patch as attachment is forthcoming.

Similar changes to /etc/rc.suspend and /etc/rc.resume are out-of-scope. Patches will be submitted separately.
Comment 1 unitrunker 2020-09-09 10:36:40 UTC
Created attachment 217835 [details]
modified /etc/rc script (not a proper patch file)
Comment 2 unitrunker 2020-09-10 05:23:04 UTC
Created attachment 217864 [details]
/etc/rc supports rcorder -p for parallel startup.
Comment 3 unitrunker 2020-09-15 16:56:31 UTC
Ouch! Forgot the '&' to run as background when I consolidated the patch file.
Comment 4 unitrunker 2020-09-15 18:20:11 UTC
Created attachment 217979 [details]
/etc/rc supports rcorder -p for parallel startup.
Comment 5 Baptiste Daroussin freebsd_committer freebsd_triage 2021-02-21 05:22:08 UTC
I am about to commit the patch, thank you for providing it!

Given we are now using git, would you mind either providing a name that should appear before your email address or directly a patch in git format-patch format ?
Comment 6 unitrunker 2021-02-22 01:24:28 UTC
Thank you Baptiste!

The name is Rick Parrish.

Regards.
Comment 7 commit-hook freebsd_committer freebsd_triage 2021-02-23 10:27:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=77e1ccbee3ed6c837929e4e232fd07f95bfc8294

commit 77e1ccbee3ed6c837929e4e232fd07f95bfc8294
Author:     Rick Parrish <unitrunker@gmail.com>
AuthorDate: 2021-02-07 06:15:21 +0000
Commit:     Baptiste Daroussin <bapt@FreeBSD.org>
CommitDate: 2021-02-23 10:16:53 +0000

    rc: implement parallel boot

    take advantage of the rcorder -p argument to implement parallel
    booting in rc.

    According to the author non scientific tests:
    on a Core 2 Duo with spinning disk:

    | Services enabled | before | after | saving |
    | 0                | 8s     | 8s    | 0      |
    | 1                | 13s    | 13s   | 0      |
    | 2                | 17s    | 13s   | 5      |
    | 3                | 23s    | 13s   | 10     |
    | 4                | 28s    | 13s   | 15     |
    | 5                | 33s    | 13s   | 20     |

    PR:             249192
    MFC after:      3 weeks

 libexec/rc/rc | 49 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 15 deletions(-)
Comment 8 Li-Wen Hsu freebsd_committer freebsd_triage 2021-02-23 11:49:12 UTC
unitrunker: do you want to also provide a patch to update rc.conf(5) about rc_parallel_start variable?
Comment 9 Daniel Ebdrup Jensen freebsd_committer freebsd_triage 2021-02-23 15:10:28 UTC
I created review D28898.
Comment 10 commit-hook freebsd_committer freebsd_triage 2021-02-23 21:26:47 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=408edcca0746d15303e0b47bbf31a6d4aa721363

commit 408edcca0746d15303e0b47bbf31a6d4aa721363
Author:     Daniel Ebdrup Jensen <debdrup@FreeBSD.org>
AuthorDate: 2021-02-23 21:25:44 +0000
Commit:     Daniel Ebdrup Jensen <debdrup@FreeBSD.org>
CommitDate: 2021-02-23 21:26:31 +0000

    rc.conf(5): Add note about parallel startup variable

    The commit below added parallel service startup, and it needs to be
    documented, so people know about it.

    PR:             249192
    MFC with:       77e1ccbee3ed

    Reviewed by:    yuripv
    Differential Revision:  https://reviews.freebsd.org/D28898

 share/man/man5/rc.conf.5 | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Comment 11 commit-hook freebsd_committer freebsd_triage 2021-02-24 05:23:14 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=6e822e99570fdf4c564be04840a054bccc070222

commit 6e822e99570fdf4c564be04840a054bccc070222
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2021-02-24 05:12:49 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2021-02-24 05:12:49 +0000

    rc: fix parse of $local_startup

    77e1ccbee3ed6c837929e4e232fd07f95bfc8294 introduced parallel execution
    of rc. It separated groups with line feeds (\n) and elements within
    groups using spaces. This is a natural separation due to rcorder
    using spaces and lines to separate elements within groups with groups
    of services separated by line feeds.

    77e1ccbee3ed6c837929e4e232fd07f95bfc8294 parses the output from rcorder
    by setting $IFS. However it failed to reset $IFS to default ' \t\n'
    prior to calling find_local_scripts_new(), causing find_local_scripts_new()
    to fail parsing $local_startup for site-specific local rc scripts, i.e.
    ${LOCALBASE}/etc/rc.d. This caused daemons from ports and packages such
    as postfix, dovecot, nut, and others in ${LOCALBASE} not to be started.

    PR:             249192
    MFC after:      3 week
    X-MFC with:     77e1ccbee3ed6c837929e4e232fd07f95bfc8294

 libexec/rc/rc | 1 +
 1 file changed, 1 insertion(+)
Comment 12 Katsuyuki Miyoshi 2021-02-25 18:18:56 UTC
/etc/rc.d/ldconfig fails.

== boot message ===============
......
Setting hostname: foo.
cat: /usr/local/libdata/ldconfig/signon-plugin-oauth2
/usr/local/libdata/ldconfig/portupgrade-devel
/usr/local/libdata/ldconfig/pkg
/usr/local/libdata/ldconfig/perl5
/usr/local/libdata/ldconfig/xrdp
/usr/local/libdata/ldconfig/opencollada
/usr/local/libdata/ldconfig/llvm11
/usr/local/libdata/ldconfig/samba411
/usr/local/libdata/ldconfig/gcc10
/usr/local/libdata/ldconfig/llvm10
/usr/local/libdata/ldconfig/gcc48
/usr/local/libdata/ldconfig/mysql57-server
/usr/local/libdata/ldconfig/llvm90
/usr/local/libdata/ldconfig/signon-kwallet-extension
/usr/local/libdata/ldconfig/mysql57-client
/usr/local/libdata/ldconfig/qt5-core: No such file or directory
ELF ldconfig path: /lib /usr/lib /usr/lib/compat /usr/local/lib
32-bit compatibility ldconfig path: /usr/lib32
......
===============================


=== /etc/rc.d/ldconfig ========
...
29:          _files=`find ${i} -type f`
30:          if [ -n "${_files}" ]; then
31:              ldconfig_paths="${ldconfig_paths} `cat ${_files} | sort -u`"
32:          fi
...
==============================
An error occurs on line 31 due to $IFS=$' ' in /etc/rc.

I think the same error is occurring at least in /etc/rc.d/{devmatch,ntpdate}.
Comment 13 Daniel Ebdrup Jensen freebsd_committer freebsd_triage 2021-02-25 18:40:27 UTC
(In reply to Katsuyuki Miyoshi from comment #12)
What version is this on?

The comment just above specifically references unsetting IFS, so are you sure you've got that one too?
Comment 14 Cy Schubert freebsd_committer freebsd_triage 2021-02-25 18:53:12 UTC
Created attachment 222830 [details]
$' ' is incorrect, it must include all whitespace

$' ' is incorrect, it must include all whitespace. Can you try this patch?
Comment 15 Cy Schubert freebsd_committer freebsd_triage 2021-02-25 19:04:33 UTC
That fixes it.
Comment 16 commit-hook freebsd_committer freebsd_triage 2021-02-25 20:07:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f1ab799927c8e93e8f58e5039f287a2ca45675ec

commit f1ab799927c8e93e8f58e5039f287a2ca45675ec
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2021-02-25 19:04:50 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2021-02-25 19:47:56 +0000

    rc: fix rc script parsing

    77e1ccbee3ed6c837929e4e232fd07f95bfc8294 introduced a bug whereby
    rc scripts in etc/rc.d and $local_startup failed to parse output
    from called commands because IFS was set to " " instead of the
    default " \t\n". This caused parsing of output that contains any
    whitespace character, such as tabs and newlines, not matching just a
    space to fail.

    PR:             249192
    MFC after:      3 weeks
    X-MFC with:     77e1ccbee3ed6c837929e4e232fd07f95bfc8294

 libexec/rc/rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 17 Katsuyuki Miyoshi 2021-02-26 00:17:18 UTC
(In reply to commit-hook from comment #16)
Dear All,
It's fine. Thanks.
Comment 18 commit-hook freebsd_committer freebsd_triage 2021-02-26 06:04:45 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=763db58932874bb47fc6f9322ab81cc947f80991

commit 763db58932874bb47fc6f9322ab81cc947f80991
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2021-02-26 05:39:18 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2021-02-26 06:03:38 +0000

    rc: save and restore $IFS

    Fix another bug in 77e1ccbee3ed6c837929e4e232fd07f95bfc8294. $IFS
    should be fully restored for its other users.

    PR:             249192
    Reported by:    jkim
    MFC after:      3 weeks
    X-MFC with:     77e1ccbee3ed6c837929e4e232fd07f95bfc8294

 libexec/rc/rc | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Comment 19 unitrunker 2023-12-16 03:20:26 UTC
This flat out did not work so abandoning.
Comment 20 Ken DEGUCHI 2023-12-16 05:16:49 UTC
Created attachment 247078 [details]
parallel startup /etc/rc

It seems to fail when exiting the first loop, so I tried "break 2" to exit.
It works fine in my environment (stable/14-n265990-a9262d053b63-dirty).