Bug 282255 - rc: Is rc_fast_and_loose actually working?
Summary: rc: Is rc_fast_and_loose actually working?
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Only Me
Assignee: Mateusz Piotrowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-10-21 17:45 UTC by Mateusz Piotrowski
Modified: 2024-10-28 21:12 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-21 17:45:24 UTC
The review of https://reviews.freebsd.org/D46862 prompted the question if rc_fast_and_loose is actually working at the moment.

rc_fast_and_loose makes rc(8) execute rc scripts by sourcing them instead of running them in subshells. This is probably broken as many of rc services call exit for example (also with the exit code of 0).

There were some discussions about rc_fast_and_loose in the past, questioning if it was properly supported or even working at all: https://lists.freebsd.org/pipermail/freebsd-current/2012-January/031167.html

We should investigate if rc_fast_and_loose is working at all.
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2024-10-21 17:47:20 UTC
From NetBSD:

     rc_fast_and_loose
                     If set to a non-empty string, each script in /etc/rc.d
                     will be executed in the current shell rather than a sub
                     shell.  This may be faster on slow machines that have an
                     expensive fork(2) operation.

                     Note:   Use this at your own risk!  A rogue command or
                             script may inadvertently prevent boot to
                             multiuser.

I don't think there's any value in keeping it these days.
Comment 2 Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-21 18:12:47 UTC
(In reply to Mark Johnston from comment #1)

I recall that a couple of years ago having rc(8) perform less syscalls would allow those running FreeBSD in emulators to get faster boot times. I've never verified if that is really the case.
Comment 3 Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-21 19:24:52 UTC
(In reply to Mateusz Piotrowski from comment #2)

Here's an email supporting the claim that rc_fast_and_loose is meant to speed up rc on slow machines:

https://lists.freebsd.org/pipermail/freebsd-rc/2011-December/002622.html
Comment 4 Brooks Davis freebsd_committer freebsd_triage 2024-10-21 19:30:08 UTC
(In reply to Mateusz Piotrowski from comment #2)

That's true, but the cost of a sub shell isn't likely the big deal.  At least for Morello under qemu, devd calling "service devmatch ..." a ton of times is the dominate cost.
Comment 5 Mark Johnston freebsd_committer freebsd_triage 2024-10-21 19:34:42 UTC
(In reply to Mateusz Piotrowski from comment #3)
I'm quite skeptical of the value of that optimization in 2024, especially if it's untested.  Consider also that NetBSD runs on a much larger variety of systems than FreeBSD; we have much higher system requirements.

As Brooks notes, some measurement should be done to indicate that forking shells really is a significant contributor to boot times.  Colin's recent work indicates that the largest gains are to be had by making sure we don't sleep more than necessary during boot.
Comment 6 Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-23 10:16:41 UTC
With rc_fast_and_loose=yes,rc(8) on my system stops processing rc services as early as rc.d/hostid, which calls exit 0. That's before my machine even sets its hostname properly. As a result, I get a login prompt saying that I've booted Amnesiac. I cannot even log in because I get "pam_open_session(): Sessions failure".

Technically, we could fix all rc service scripts in base to be compatible with rc_fast_and_loose, but it feels unreasonable. Firstly, this is going to be an up hill battle in the ports tree. Secondly, being compatible with rc_fast_and_loose requires rc service scripts to use unusual idioms like `(exit 1)` to set the return code without actually exiting if sourced.

My recommendation is to remove support for rc_fast_and_loose.
Comment 7 Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-23 13:32:00 UTC
Here's a patch removing rc_fast_and_loose: https://reviews.freebsd.org/D47264
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-10-28 21:12:05 UTC
A commit in branch main references this bug:

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

commit a5ad360ff9b7fb2ef3f7f31b8c29c332026b0e01
Author:     Mateusz Piotrowski <0mp@FreeBSD.org>
AuthorDate: 2024-10-23 12:57:29 +0000
Commit:     Mateusz Piotrowski <0mp@FreeBSD.org>
CommitDate: 2024-10-28 21:10:49 +0000

    rc: Remove rc_fast_and_loose

    The rc_fast_and_loose variable allowed rc(8) to start services
    by sourcing them into rc's own shell environment. Normally, each rc
    service script is started by being sourced into its own subshell
    instead.  The feature was meant to speed up rc(8) by avoiding the extra
    forking necessary to spawn subshells.

    In practice, the feature has been broken for a long time now. One of the
    reasons is that some rc service scripts call the exit builtin to return
    non-zero error codes, which not only terminates the service subshell
    but also rc(8) when rc_fast_and_loose is enabled. For example,
    a system running any of the supported FreeBSD releases
    with rc_fast_and_loose=yes would abort rc(8) as early as rc.d/hostid,
    due to an "exit 0".

    Fixing rc_fast_and_loose support would require rewriting some rc scripts
    to support being sourced directly into rc(8) process. This would muddy
    the code base and also would prove difficult to maintain long term
    as this is simply not how rc(8) users write scripts. The potential
    performance benefits are unlikely to be significant even for use cases
    such as Morello under qemu.

    Instead, remove support for rc_fast_and_loose completely from rc(8)
    and inform users about the change.

    PR:             282255
    Reviewed by:    brooks, christos, mhorne
    Approved by:    christos (mentor), markj (mentor)
    MFC after:      2 weeks
    Relnotes:       yes
    Differential Revision:  https://reviews.freebsd.org/D47264

 UPDATING                 |  7 +++++++
 libexec/rc/rc.subr       | 20 ++------------------
 share/man/man8/rc.subr.8 | 36 +++++++++++-------------------------
 3 files changed, 20 insertions(+), 43 deletions(-)
Comment 9 Mateusz Piotrowski freebsd_committer freebsd_triage 2024-10-28 21:12:35 UTC
Thanks!